From: "Paul E. McKenney" <paulmck@kernel.org>
To: Dmitry Vyukov <dvyukov@google.com>
Cc: "Xu, Yanfei" <yanfei.xu@windriver.com>,
syzbot <syzbot+7b2b13f4943374609532@syzkaller.appspotmail.com>,
rcu@vger.kernel.org, Andrew Morton <akpm@linux-foundation.org>,
Andrii Nakryiko <andrii@kernel.org>,
Alexei Starovoitov <ast@kernel.org>, Jens Axboe <axboe@kernel.dk>,
bpf <bpf@vger.kernel.org>,
Christian Brauner <christian@brauner.io>,
Daniel Borkmann <daniel@iogearbox.net>,
John Fastabend <john.fastabend@gmail.com>,
Martin KaFai Lau <kafai@fb.com>, KP Singh <kpsingh@kernel.org>,
LKML <linux-kernel@vger.kernel.org>,
netdev <netdev@vger.kernel.org>,
Shakeel Butt <shakeelb@google.com>,
Song Liu <songliubraving@fb.com>,
syzkaller-bugs <syzkaller-bugs@googlegroups.com>,
Yonghong Song <yhs@fb.com>
Subject: Re: [syzbot] KASAN: use-after-free Read in check_all_holdout_tasks_trace
Date: Tue, 25 May 2021 07:36:32 -0700 [thread overview]
Message-ID: <20210525143632.GP4441@paulmck-ThinkPad-P17-Gen-1> (raw)
In-Reply-To: <CACT4Y+bkZv7uo505EBJcE1MLFG1GprZ5npdbaUXZ+ASTJyJU8A@mail.gmail.com>
On Tue, May 25, 2021 at 10:33:27AM +0200, Dmitry Vyukov wrote:
> On Tue, May 25, 2021 at 5:33 AM Paul E. McKenney <paulmck@kernel.org> wrote:
> >
> > On Tue, May 25, 2021 at 10:31:55AM +0800, Xu, Yanfei wrote:
> > >
> > >
> > > On 5/25/21 6:46 AM, Paul E. McKenney wrote:
> > > > [Please note: This e-mail is from an EXTERNAL e-mail address]
> > > >
> > > > On Sun, May 23, 2021 at 09:13:50PM -0700, Paul E. McKenney wrote:
> > > > > On Sun, May 23, 2021 at 08:51:56AM +0200, Dmitry Vyukov wrote:
> > > > > > On Fri, May 21, 2021 at 7:29 PM syzbot
> > > > > > <syzbot+7b2b13f4943374609532@syzkaller.appspotmail.com> wrote:
> > > > > > >
> > > > > > > Hello,
> > > > > > >
> > > > > > > syzbot found the following issue on:
> > > > > > >
> > > > > > > HEAD commit: f18ba26d libbpf: Add selftests for TC-BPF management API
> > > > > > > git tree: bpf-next
> > > > > > > console output: https://syzkaller.appspot.com/x/log.txt?x=17f50d1ed00000
> > > > > > > kernel config: https://syzkaller.appspot.com/x/.config?x=8ff54addde0afb5d
> > > > > > > dashboard link: https://syzkaller.appspot.com/bug?extid=7b2b13f4943374609532
> > > > > > >
> > > > > > > Unfortunately, I don't have any reproducer for this issue yet.
> > > > > > >
> > > > > > > IMPORTANT: if you fix the issue, please add the following tag to the commit:
> > > > > > > Reported-by: syzbot+7b2b13f4943374609532@syzkaller.appspotmail.com
> > > > > >
> > > > > > This looks rcu-related. +rcu mailing list
> > > > >
> > > > > I think I see a possible cause for this, and will say more after some
> > > > > testing and after becoming more awake Monday morning, Pacific time.
> > > >
> > > > No joy. From what I can see, within RCU Tasks Trace, the calls to
> > > > get_task_struct() are properly protected (either by RCU or by an earlier
> > > > get_task_struct()), and the calls to put_task_struct() are balanced by
> > > > those to get_task_struct().
> > > >
> > > > I could of course have missed something, but at this point I am suspecting
> > > > an unbalanced put_task_struct() has been added elsewhere.
> > > >
> > > > As always, extra eyes on this code would be a good thing.
> > > >
> > > > If it were reproducible, I would of course suggest bisection. :-/
> > > >
> > > > Thanx, Paul
> > > >
> > > Hi Paul,
> > >
> > > Could it be?
> > >
> > > CPU1 CPU2
> > > trc_add_holdout(t, bhp)
> > > //t->usage==2
> > > release_task
> > > put_task_struct_rcu_user
> > > delayed_put_task_struct
> > > ......
> > > put_task_struct(t)
> > > //t->usage==1
> > >
> > > check_all_holdout_tasks_trace
> > > ->trc_wait_for_one_reader
> > > ->trc_del_holdout
> > > ->put_task_struct(t)
> > > //t->usage==0 and task_struct freed
> > > READ_ONCE(t->trc_reader_checked)
> > > //ops, t had been freed.
> > >
> > > So, after excuting trc_wait_for_one_reader(), task might had been removed
> > > from holdout list and the corresponding task_struct was freed.
> > > And we shouldn't do READ_ONCE(t->trc_reader_checked).
> >
> > I was suspicious of that call to trc_del_holdout() from within
> > trc_wait_for_one_reader(), but the only time it executes is in the
> > context of the current running task, which means that CPU 2 had better
> > not be invoking release_task() on it just yet.
> >
> > Or am I missing your point?
> >
> > Of course, if you can reproduce it, the following patch might be
> > an interesting thing to try, my doubts notwithstanding. But more
> > important, please check the patch to make sure that we are both
> > talking about the same call to trc_del_holdout()!
> >
> > If we are talking about the same call to trc_del_holdout(), are you
> > actually seeing that code execute except when rcu_tasks_trace_pertask()
> > calls trc_wait_for_one_reader()?
> >
> > > I investigate the trc_wait_for_one_reader() and found before we excute
> > > trc_del_holdout, there is always set t->trc_reader_checked=true. How about
> > > we just set the checked flag and unified excute trc_del_holdout()
> > > in check_all_holdout_tasks_trace with checking the flag?
> >
> > The problem is that we cannot execute trc_del_holdout() except in
> > the context of the RCU Tasks Trace grace-period kthread. So it is
> > necessary to manipulate ->trc_reader_checked separately from the list
> > in order to safely synchronize with IPIs and with the exit code path
> > for any reader tasks, see for example trc_read_check_handler() and
> > exit_tasks_rcu_finish_trace().
> >
> > Or are you thinking of some other approach?
>
> This could be caused by a buggy extra put_pid somewhere else, right?
> If so, I suspect that's what may be happening. We've 2 very similar
> use-after-free reports on an internal kernel, but it also has a number
> of other use-after-free reports in pid-related functions
> (pid_task/pid_nr_ns/attach_pid). One of them is happening relatively
> frequently (150 crashes) and is caused by something in the tty
> subsystem. Presumably it may be causing one off use-after-free's in
> other random places of the kernel as well. Unfortunately these crashes
> don't happen on the upstream kernel (at least not yet).
> So if you don't see any obvious smoking gun in rcu, I think we can
> assume for now that it's due to tty.
Good to hear!
On the other hand, it looks like Yanfei might have found a real problem,
whether or not it was actually being triggered. Tough to prove either
way, of course!
Thanx, Paul
> > ------------------------------------------------------------------------
> >
> > diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
> > index efb8127f3a36..2a0d4bdd619a 100644
> > --- a/kernel/rcu/tasks.h
> > +++ b/kernel/rcu/tasks.h
> > @@ -987,7 +987,6 @@ static void trc_wait_for_one_reader(struct task_struct *t,
> > // The current task had better be in a quiescent state.
> > if (t == current) {
> > t->trc_reader_checked = true;
> > - trc_del_holdout(t);
> > WARN_ON_ONCE(READ_ONCE(t->trc_reader_nesting));
> > return;
> > }
> >
> > --
> > You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bugs+unsubscribe@googlegroups.com.
> > To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/20210525033355.GN4441%40paulmck-ThinkPad-P17-Gen-1.
next prev parent reply other threads:[~2021-05-25 14:37 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-21 17:29 [syzbot] KASAN: use-after-free Read in check_all_holdout_tasks_trace syzbot
2021-05-23 6:51 ` Dmitry Vyukov
2021-05-24 4:13 ` Paul E. McKenney
2021-05-24 22:46 ` Paul E. McKenney
2021-05-25 2:31 ` Xu, Yanfei
2021-05-25 3:33 ` Paul E. McKenney
2021-05-25 8:33 ` Dmitry Vyukov
2021-05-25 14:36 ` Paul E. McKenney [this message]
2021-05-25 10:24 ` Xu, Yanfei
2021-05-25 14:28 ` Paul E. McKenney
2021-05-26 2:22 ` Xu, Yanfei
2021-05-26 4:21 ` Paul E. McKenney
2021-05-26 6:03 ` Xu, Yanfei
2021-06-18 20:45 ` syzbot
2021-06-21 22:37 ` Paul E. McKenney
2021-06-19 16:54 ` syzbot
2021-06-21 22:41 ` Paul E. McKenney
2021-06-28 8:43 ` Dmitry Vyukov
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=20210525143632.GP4441@paulmck-ThinkPad-P17-Gen-1 \
--to=paulmck@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=axboe@kernel.dk \
--cc=bpf@vger.kernel.org \
--cc=christian@brauner.io \
--cc=daniel@iogearbox.net \
--cc=dvyukov@google.com \
--cc=john.fastabend@gmail.com \
--cc=kafai@fb.com \
--cc=kpsingh@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=rcu@vger.kernel.org \
--cc=shakeelb@google.com \
--cc=songliubraving@fb.com \
--cc=syzbot+7b2b13f4943374609532@syzkaller.appspotmail.com \
--cc=syzkaller-bugs@googlegroups.com \
--cc=yanfei.xu@windriver.com \
--cc=yhs@fb.com \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).