linux-kernel - Re: Oops in 3.7-rc8 isolate_free_pages

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: <CA+55aFw9WQN-MYFKzoGXF9Z70h1XsMu5X4hLy0GPJopBVuE=Yg@mail.gmail.com>
Date:	Thu, 6 Dec 2012 08:50:54 -0800
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Mel Gorman <mgorman@...e.de>
Cc:	Jan Kara <jack@...e.cz>, Henrik Rydberg <rydberg@...omail.se>,
	linux-mm <linux-mm@...ck.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: Oops in 3.7-rc8 isolate_free_pages_block()

On Thu, Dec 6, 2012 at 8:19 AM, Mel Gorman <mgorman@...e.de> wrote:
>
> Still travelling and am not in a position to test this properly :(.
> However, this bug feels very similar to a bug in the migration scanner where
> a pfn_valid check is missed because the start is not aligned.

Ugh. This patch makes my eyes bleed.

Is there no way to do this nicely in the caller? IOW, fix the
'end_pfn' logic way upstream where it is computed, and just cap it at
the MAX_ORDER_NR_PAGES boundary?

For example, isolate_freepages_range() seems to have this *other*
end-point alignment thing going on, and does it in a loop. Wouldn't it
be much better to have a separate loop that looped up to the next
MAX_ORDER_NR_PAGES boundary instead of having this kind of very random
test in the middle of a loop.

Even the name ("isolate_freepages_block") implies that we have a
"block" of pages. Having to have a random "oops, this block can have
other blocks inside of it that aren't mapped" test in the middle of
that function really makes me go "Uhh, no".

Plus, is it even guaranteed that the *first* pfn (that we get called
with) is pfnvalid to begin with?

So I guess this patch fixes things, but it does make me go "That's
really *really* ugly".

                  Linus
--
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/