linux-kernel - Re: linux-next: Tree for Jan 20 -- sparc32: fix broken set

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <54C150BB.6050000@roeck-us.net>
Date:	Thu, 22 Jan 2015 11:34:19 -0800
From:	Guenter Roeck <linux@...ck-us.net>
To:	"Kirill A. Shutemov" <kirill@...temov.name>, davem@...emloft.net
CC:	Mel Gorman <mgorman@...e.de>,
	Stephen Rothwell <sfr@...b.auug.org.au>,
	linux-next@...r.kernel.org, linux-kernel@...r.kernel.org,
	Paul Moore <pmoore@...hat.com>,
	Joonsoo Kim <iamjoonsoo.kim@....com>
Subject: Re: linux-next: Tree for Jan 20 -- sparc32: fix broken set_pte()

On 01/22/2015 09:13 AM, Kirill A. Shutemov wrote:
...
> vm_normal_page() is never called in this case, since prot_numa is always
> zero.
>
> I tracked the bug down. It's a sparc bug. The commit only triggers it,
> because affect how GCC optimize the code around faulty point.
>
> Please, test.
>
>>>From 5b9232753217412116a4cdc2897be0db818371ca Mon Sep 17 00:00:00 2001
> From: "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
> Date: Thu, 22 Jan 2015 18:42:13 +0200
> Subject: [PATCH] sparc32: fix broken set_pte()
>
> 32-bit sparc uses swap instruction to implement set_pte(). It called
> using GCC inline assembler. But it misses the "memory" clobber to
> indicate that pte value will be updated in memory.
>
> As result GCC doesn't know that it cannot postpone pte pointer
> dereference which occurs before set_pte() to post-set_pte() time.
>
> It leads to real-world bugs -- [1]. In this situation we have code:
>
> 	ptent = ptep_modify_prot_start(mm, addr, pte);
> 	ptent = pte_modify(ptent, newprot);
> 	...
> 	ptep_modify_prot_commit(mm, addr, pte, ptent);
>
> ptep_modify_prot_start() in sparc case is just 'pte' dereference plus
> pte_clear(). pte_clear() calls broken set_pte(). GCC thinks it's valid
> to dereference 'pte' again on pte_modify() and gets cleared pte.
> ptep_modify_prot_commit() puts 'pteent' with pfn==0 back to page table,
> which eventually leads to the crash.
>
> [1] http://lkml.kernel.org/r/54C06B19.8060305@roeck-us.net
>
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@...ux.intel.com>
> Reported-by: Guenter Roeck <linux@...ck-us.net>

Excellent catch. Yes, the fix works.

Tested-by: Guenter Roeck <linux@...ck-us.net>

Thanks,
Guenter

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/