Re: Patch "mm/kmemleak.c: wait for scan completion before disabling free" has been added to the 3.18-stable tree

[Greg KH <gregkh@linuxfoundation.org>]

On Mon, May 07, 2018 at 01:22:36PM +0530, Vinayak Menon wrote:
> 
> On 5/3/2018 2:13 AM, gregkh@linuxfoundation.org wrote:
> > This is a note to let you know that I've just added the patch titled
> >
> >     mm/kmemleak.c: wait for scan completion before disabling free
> >
> > to the 3.18-stable tree which can be found at:
> >     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> >
> > The filename of the patch is:
> >      mm-kmemleak.c-wait-for-scan-completion-before-disabling-free.patch
> > and it can be found in the queue-3.18 subdirectory.
> >
> > If you, or anyone else, feels it should not be added to the stable tree,
> > please let <stable@vger.kernel.org> know about it.
> >
> >
> > From foo@baz Wed May  2 13:21:44 PDT 2018
> > From: Vinayak Menon <vinmenon@codeaurora.org>
> > Date: Wed, 28 Mar 2018 16:01:16 -0700
> > Subject: mm/kmemleak.c: wait for scan completion before disabling free
> >
> > From: Vinayak Menon <vinmenon@codeaurora.org>
> >
> > [ Upstream commit 914b6dfff790544d9b77dfd1723adb3745ec9700 ]
> >
> > A crash is observed when kmemleak_scan accesses the object->pointer,
> > likely due to the following race.
> >
> >   TASK A             TASK B                     TASK C
> >   kmemleak_write
> >    (with "scan" and
> >    NOT "scan=on")
> >   kmemleak_scan()
> >                      create_object
> >                      kmem_cache_alloc fails
> >                      kmemleak_disable
> >                      kmemleak_do_cleanup
> >                      kmemleak_free_enabled = 0
> >                                                 kfree
> >                                                 kmemleak_free bails out
> >                                                  (kmemleak_free_enabled is 0)
> >                                                 slub frees object->pointer
> >   update_checksum
> >   crash - object->pointer
> >    freed (DEBUG_PAGEALLOC)
> >
> > kmemleak_do_cleanup waits for the scan thread to complete, but not for
> > direct call to kmemleak_scan via kmemleak_write.  So add a wait for
> > kmemleak_scan completion before disabling kmemleak_free, and while at it
> > fix the comment on stop_scan_thread.
> >
> > [vinmenon@codeaurora.org: fix stop_scan_thread comment]
> >   Link: http://lkml.kernel.org/r/1522219972-22809-1-git-send-email-vinmenon@codeaurora.org
> > Link: http://lkml.kernel.org/r/1522063429-18992-1-git-send-email-vinmenon@codeaurora.org
> > Signed-off-by: Vinayak Menon <vinmenon@codeaurora.org>
> > Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> > Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> > Signed-off-by: Sasha Levin <alexander.levin@microsoft.com>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> >  mm/kmemleak.c |   12 +++++++-----
> >  1 file changed, 7 insertions(+), 5 deletions(-)
> >
> > --- a/mm/kmemleak.c
> > +++ b/mm/kmemleak.c
> > @@ -1481,8 +1481,7 @@ static void start_scan_thread(void)
> >  }
> >  
> >  /*
> > - * Stop the automatic memory scanning thread. This function must be called
> > - * with the scan_mutex held.
> > + * Stop the automatic memory scanning thread.
> >   */
> >  static void stop_scan_thread(void)
> >  {
> > @@ -1746,12 +1745,15 @@ static void kmemleak_do_cleanup(struct w
> >  	mutex_lock(&scan_mutex);
> >  	stop_scan_thread();
> >  
> > +	mutex_lock(&scan_mutex);
> 
> The lock is taken once above� stop_scan_thread() and can result in a deadlock.
> 
> The lock was removed from kmemleak_do_cleanup by following commit which is not present on 3.18
> 
> commit 5f369f374ba4889fe3c17883402db5ee8d254216
> Author: Catalin Marinas <catalin.marinas@arm.com>
> Date:�� Wed Jun 24 16:58:31 2015 -0700
> 
> ��� mm: kmemleak: do not acquire scan_mutex in kmemleak_do_cleanup()
> 
> So we may have to pick the above patch too.

All was dropped, so no worries anymore :)

thanks,

greg k-h