Re: [PATCH 2/3] mm/vmalloc: do not call kmemleak_free() on not yet accounted memory

[Roman Penyaev]

On 2019-01-11 20:26, Andrey Ryabinin wrote:
> On 1/3/19 5:59 PM, Roman Penyaev wrote:
> > __vmalloc_area_node() calls vfree() on error path, which in turn calls
> > kmemleak_free(), but area is not yet accounted by kmemleak_vmalloc().
> >
> > Signed-off-by: Roman Penyaev <rpenyaev@xxxxxxx>
> > Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> > Cc: Michal Hocko <mhocko@xxxxxxxx>
> > Cc: Andrey Ryabinin <aryabinin@xxxxxxxxxxxxx>
> > Cc: Joe Perches <joe@xxxxxxxxxxx>
> > Cc: "Luis R. Rodriguez" <mcgrof@xxxxxxxxxx>
> > Cc: linux-mm@xxxxxxxxx
> > Cc: linux-kernel@xxxxxxxxxxxxxxx
> > ---
> >  mm/vmalloc.c | 16 +++++++++++-----
> >  1 file changed, 11 insertions(+), 5 deletions(-)
> >
> > diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> > index 2cd24186ba84..dc6a62bca503 100644
> > --- a/mm/vmalloc.c
> > +++ b/mm/vmalloc.c
> > @@ -1565,6 +1565,14 @@ void vfree_atomic(const void *addr)
> >  	__vfree_deferred(addr);
> >  }
> >
> > +static void __vfree(const void *addr)
> > +{
> > +	if (unlikely(in_interrupt()))
> > +		__vfree_deferred(addr);
> > +	else
> > +		__vunmap(addr, 1);
> > +}
> > +
> >  /**
> >   *	vfree  -  release memory allocated by vmalloc()
> >   *	@addr:		memory base address
> > @@ -1591,10 +1599,8 @@ void vfree(const void *addr)
> >
> >  	if (!addr)
> >  		return;
> > -	if (unlikely(in_interrupt()))
> > -		__vfree_deferred(addr);
> > -	else
> > -		__vunmap(addr, 1);
> > +
> > +	__vfree(addr);
> >  }
> >  EXPORT_SYMBOL(vfree);
> >
> > @@ -1709,7 +1715,7 @@ static void *__vmalloc_area_node(struct 
> > vm_struct *area, gfp_t gfp_mask,
> >  	warn_alloc(gfp_mask, NULL,
> >  			  "vmalloc: allocation failure, allocated %ld of %ld bytes",
> >  			  (area->nr_pages*PAGE_SIZE), area->size);
> > -	vfree(area->addr);
> > +	__vfree(area->addr);
>
> This can't be an interrupt context for a several reasons. One of them
> is BUG_ON(in_interrupt()) in __get_vm_area_node()
> which is called right before __vmalloc_are_node().
>
> So you can just do __vunmap(area->addr, 1); instead of __vfree().

Thanks, I missed that BUG_ON and could not prove, that we can call only
from a task context, thus decided not to make it strict.  Of course
simple __vunmap() is much better.  The other reason is that we call a
spin_lock without disabling the interrupts.  Now I see.

Andrew, may I resend just an updated version of this patch?

--
Roman