All of lore.kernel.org
 help / color / mirror / Atom feed
From: Qian Cai <cai@lca.pw>
To: peterz@infradead.org
Cc: mark.rutland@arm.com, akpm@linux-foundation.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH -next] fork: silence a false postive warning in __mmdrop
Date: Wed, 22 Jul 2020 09:19:00 -0400	[thread overview]
Message-ID: <20200722131900.GA4041@lca.pw> (raw)
In-Reply-To: <20200722100637.GR119549@hirez.programming.kicks-ass.net>

On Wed, Jul 22, 2020 at 12:06:37PM +0200, peterz@infradead.org wrote:
> On Thu, Jun 04, 2020 at 11:03:44AM -0400, Qian Cai wrote:
> > The linux-next commit bf2c59fce407 ("sched/core: Fix illegal RCU from
> > offline CPUs") delayed,
> > 
> > idle->active_mm = &init_mm;
> > 
> > into finish_cpu() instead of idle_task_exit() which results in a false
> > positive warning that was originally designed in the commit 3eda69c92d47
> > ("kernel/fork.c: detect early free of a live mm").
> > 
> >  WARNING: CPU: 127 PID: 72976 at kernel/fork.c:697
> >  __mmdrop+0x230/0x2c0
> >  do_exit+0x424/0xfa0
> >  Call Trace:
> >  do_exit+0x424/0xfa0
> >  do_group_exit+0x64/0xd0
> >  sys_exit_group+0x24/0x30
> >  system_call_exception+0x108/0x1d0
> >  system_call_common+0xf0/0x278
> 
> Please explain; because afaict this is a use-after-free.
> 
> The thing is __mmdrop() is going to actually free the mm, so then what
> is finish_cpu()'s mmdrop() going to do?
> 
> ->active_mm() should have a refcount on the mm.

Well, the refcount issue you mentioned then happens all before bf2c59fce407 was
introduced as well, but then it looks harmless because mmdrop() in finish_cpu()
will do,

	if (unlikely(atomic_dec_and_test(&mm->mm_count)))
		__mmdrop(mm);

where that atomic_dec_and_test() see the negative refcount and will not involve
__mmdrop() again. It is not clear to me that once the CPU is offline if it
needs to care about its idle thread mm_count at all. Even if this refcount
issue is finally addressed, it could hit this warning in finish_cpu() without
this patch.

On the other hand, if you look at the commit 3eda69c92d47, it is clearly that
the assumption of,

   WARN_ON_ONCE(mm == current->active_mm);

is totally gone due to bf2c59fce407. Thus, the patch is to fix that discrepancy
first and then I'll look at that the imbalance mmdrop()/mmgrab() elsewhere.

> 
> > Fixes: bf2c59fce407 ("sched/core: Fix illegal RCU from offline CPUs")
> > Signed-off-by: Qian Cai <cai@lca.pw>
> > ---
> >  kernel/fork.c | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/kernel/fork.c b/kernel/fork.c
> > index 142b23645d82..5334efd2a680 100644
> > --- a/kernel/fork.c
> > +++ b/kernel/fork.c
> > @@ -694,7 +694,6 @@ void __mmdrop(struct mm_struct *mm)
> >  {
> >  	BUG_ON(mm == &init_mm);
> >  	WARN_ON_ONCE(mm == current->mm);
> > -	WARN_ON_ONCE(mm == current->active_mm);
> >  	mm_free_pgd(mm);
> >  	destroy_context(mm);
> >  	mmu_notifier_subscriptions_destroy(mm);
> > -- 
> > 2.21.0 (Apple Git-122.2)
> > 

  reply	other threads:[~2020-07-22 13:19 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-04 15:03 Qian Cai
2020-07-21 17:30 ` Qian Cai
2020-07-22 10:06 ` peterz
2020-07-22 13:19   ` Qian Cai [this message]
2020-07-22 13:44     ` Peter Zijlstra
2020-09-08 16:50       ` Qian Cai
2020-09-08 16:50         ` Qian Cai
2020-09-08 17:58         ` peterz
2020-09-08 17:58           ` peterz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200722131900.GA4041@lca.pw \
    --to=cai@lca.pw \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=peterz@infradead.org \
    --subject='Re: [PATCH -next] fork: silence a false postive warning in __mmdrop' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.