Re: mm: compaction: buffer overflow in isolate_migratepages_range

[Rafael Aquini]

On Thu, Aug 14, 2014 at 10:07:40PM +0400, Andrey Ryabinin wrote:
> 2014-08-14 19:13 GMT+04:00 Rafael Aquini <aquini@xxxxxxxxxx>:
> > It still a harmless condition as before, but considering what goes above
> > I'm now convinced & confident the patch proposed by Andrey is the real fix
> > for such occurrences.
> >
> 
> I don't think that it's harmless, because we could cross page boundary here and
> try to read from a memory hole.
>
I think isolate_migratepages_range() skips over holes, doesn't it? 


> And this code has more potential problems like use after free. Since
> we don't hold locks properly here,
> page->mapping could point to freed struct address_space.
>
Thinking on how things go for isolate_migratepages_range() and balloon
pages, I struggle to find a way where that could happen. OTOH, I failed
to see things more blatant before, so I won't argue here. Defensive
programming is always better than negating possibilities ;)

 
> We discussed this with Konstantin and he suggested a better solution for this.
> If I understood him correctly the main idea was to store bit
> identifying ballon page
> in struct page (special value in _mapcount), so we won't need to check
> mapping->flags.
>
I liked it. Something in the line of PageBuddy()/PAGE_BUDDY_MAPCOUNT_VALUE scheme.
This is clearly cleaner than what we have in place today, and I'm
ashamed I didn't think of it before. Thanks for pointing that out.

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