Re: [PATCH v2] mm, dump_page: do not crash with bad compound_mapcount()

[Vlastimil Babka]

On 8/4/20 11:48 PM, John Hubbard wrote:
> If a compound page is being split while dump_page() is being run on that
> page, we can end up calling compound_mapcount() on a page that is no
> longer compound. This leads to a crash (already seen at least once in
> the field), due to the VM_BUG_ON_PAGE() assertion inside
> compound_mapcount().
> 
> (The above is from Matthew Wilcox's analysis of Qian Cai's bug report.)
> 
> A similar problem is possible, via compound_pincount() instead of
> compound_mapcount().
> 
> In order to avoid this kind of crash, make dump_page() slightly more
> robust, by providing a pair of simpler routines that don't contain
> assertions: head_mapcount() and head_pincount().
> 
> For debug tools, we don't want to go *too* far in this direction, but
> this is a simple small fix, and the crash has already been seen, so it's
> a good trade-off.
> 
> Reported-by: Qian Cai <cai@xxxxxx>
> Suggested-by: Matthew Wilcox <willy@xxxxxxxxxxxxx>
> Cc: Vlastimil Babka <vbabka@xxxxxxx>
> Cc: Kirill A. Shutemov <kirill.shutemov@xxxxxxxxxxxxxxx>
> Signed-off-by: John Hubbard <jhubbard@xxxxxxxxxx>

Acked-by: Vlastimil Babka <vbabka@xxxxxxx>

> ---
> Hi,
> 
> I'm assuming that a fix is not required for -stable, but let me know if
> others feel differently. The dump_page() code has changed a lot in that
> area.

I'd say only if we backport all those changes, as most were to avoid similar
kind of crashes?
How about this additional patch now that we have head_mapcoun()? (I wouldn't
go for squashing as the goal and scope is too different).

----8<----