Re: [bug report] mm: avoid leaving partial pfn mappings around in error case

[Dan Carpenter <dan.carpenter@linaro.org>]

On Sun, Sep 15, 2024 at 12:23:31PM +0200, Linus Torvalds wrote:
> On Sun, 15 Sept 2024 at 12:08, Dan Carpenter <dan.carpenter@linaro.org> wrote:
> >
> > The lru_add_drain() function at the start of zap_page_range_single() takes a
> > mutex.
> 
> Yes, that shouldn't be problematic. But:
> 
> > It's the preempt_disable() in gru_fault() which is the issue.  The call tree
> > is:
> >
> > gru_fault() <- disables preempt
> > -> remap_pfn_range()
> >    -> remap_pfn_range_notrack()
> 
> That code is very odd. It was invalid to call remap_pfn_range() with
> preemption disabled even before, because it will allocate the page
> tables that it fills in.
> 
> But presumably *that* never happened in practice, and so nobody
> noticed how broken that code was before.
> 
> Now smatch seems to see a new problem, but I *think* it's because
> smatch didn't notice the sleeping by p4d_alloc() / pud_alloc() /
> pmd_alloc() because those allocations are all conditional (so smatch
> doesn't see them as static violations).
> 
> Put another way: I do not believe this is a new issue, but perhaps a
> "new to smatch" issue?
> 

Yep.  You're right.  Smatch doesn't count allocations as sleeping when we pass a
variable to for the gfp flags and those functions do "get_zeroed_page(gfp)".
I've been intending for years to handle bitmasks better but I've never
implemented that code.

regards,
dan carpenter