linux-kernel - Re: [PATCH] x86: ioremap ram check fix

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <19f34abd0804300904x7000cc6q8d717c3cd524ada4@mail.gmail.com>
Date:	Wed, 30 Apr 2008 18:04:23 +0200
From:	"Vegard Nossum" <vegard.nossum@...il.com>
To:	"Andres Salomon" <dilinger@...ued.net>
Cc:	"Jan Beulich" <jbeulich@...ell.com>, "Ingo Molnar" <mingo@...e.hu>,
	"Thomas Gleixner" <tglx@...utronix.de>,
	"H. Peter Anvin" <hpa@...or.com>,
	"Andrew Morton" <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] x86: ioremap ram check fix

On Wed, Apr 30, 2008 at 5:30 PM, Andres Salomon <dilinger@...ued.net> wrote:
> Hi Ingo,
>
>  bdd3cee2e4b7279457139058615ced6c2b41e7de (x86: ioremap(), extend check
>  to all RAM pages) breaks OLPC's ioremap call.  The ioremap that OLPC uses is:
>
>         romsig = ioremap(0xffffffc0, 16);
>
>  The commit that breaks it is basically:
>
>  -       for (pfn = phys_addr >> PAGE_SHIFT; pfn < max_pfn_mapped &&
>  -            (pfn << PAGE_SHIFT) < last_addr; pfn++) {
>  +       for (pfn = phys_addr >> PAGE_SHIFT;
>  +                               (pfn << PAGE_SHIFT) < last_addr; pfn++) {
>  +
>
>  Previously, the 'pfn < max_pfn_mapped' check would've caused us to not
>  enter the loop.  Removing that check means we loop infinitely.  The
>  reason for that is because pfn is 0xfffff, and last_addr is 0xffffffcf.
>  The remaining check that is used to exit the loop is not sufficient;
>  when pfn<<PAGE_SHIFT is 0xfffff000, that is less than 0xffffffcf; when
>  we increment pfn and it overflows (pfn == 0x100000), pfn<<PAGE_SHIFT
>  ends up being 0.  That, of course, is less than last_addr.  In effect,
>  pfn<<PAGE_SHIFT is never lower than last_addr.
>
>  The simple fix for this is to limit the last_addr check to the PAGE_MASK;
>  a patch is below.
>
>
>
>
>  Signed-off-by: Andres Salomon <dilinger@...ian.org>
>  ---
>   arch/x86/mm/ioremap.c |    3 ++-
>   1 files changed, 2 insertions(+), 1 deletions(-)
>
>  diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
>  index 804de18..fb960f5 100644
>  --- a/arch/x86/mm/ioremap.c
>  +++ b/arch/x86/mm/ioremap.c
>  @@ -149,7 +149,8 @@ static void __iomem *__ioremap_caller(resource_size_t phys_addr,
>          * Don't allow anybody to remap normal RAM that we're using..
>          */
>         for (pfn = phys_addr >> PAGE_SHIFT;
>  -                               (pfn << PAGE_SHIFT) < last_addr; pfn++) {
>  +                               (pfn << PAGE_SHIFT) < (last_addr & PAGE_MASK);
>  +                               pfn++) {
>
>                 int is_ram = page_is_ram(pfn);
>

This fixes two 150-second solid freezes during bootup on a P4 I have
as well. Acked!

Vegard

-- 
"The animistic metaphor of the bug that maliciously sneaked in while
the programmer was not looking is intellectually dishonest as it
disguises that the error is the programmer's own creation."
	-- E. W. Dijkstra, EWD1036
--
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/