linux-kernel - Re: [PATCH] rcu/nocb: Add Safe checks for access offloaded rdp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CALm+0cWvx1N1KfGJU=kQoTaBRRf9trEU2TzL40b-gXHv5H+Utg@mail.gmail.com>
Date: Wed, 30 Apr 2025 22:54:19 +0800
From: Z qiang <qiang.zhang1211@...il.com>
To: Frederic Weisbecker <frederic@...nel.org>
Cc: paulmck@...nel.org, neeraj.upadhyay@...nel.org, joel@...lfernandes.org, 
	urezki@...il.com, boqun.feng@...il.com, rcu@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] rcu/nocb: Add Safe checks for access offloaded rdp

>
> >
> > Le Mon, Apr 28, 2025 at 05:54:03PM +0800, Zqiang a écrit :
> > > For Preempt-RT kernel, when enable CONFIG_PROVE_RCU Kconfig,
> > > disable local bh in rcuc kthreads will not affect preempt_count(),
> > > this resulted in the following splat:
> > >
> > > WARNING: suspicious RCU usage
> > > kernel/rcu/tree_plugin.h:36 Unsafe read of RCU_NOCB offloaded state!
> > > stack backtrace:
> > > CPU: 0 UID: 0 PID: 22 Comm: rcuc/0
> > > Call Trace:
> > > [    0.407907]  <TASK>
> > > [    0.407910]  dump_stack_lvl+0xbb/0xd0
> > > [    0.407917]  dump_stack+0x14/0x20
> > > [    0.407920]  lockdep_rcu_suspicious+0x133/0x210
> > > [    0.407932]  rcu_rdp_is_offloaded+0x1c3/0x270
> > > [    0.407939]  rcu_core+0x471/0x900
> > > [    0.407942]  ? lockdep_hardirqs_on+0xd5/0x160
> > > [    0.407954]  rcu_cpu_kthread+0x25f/0x870
> > > [    0.407959]  ? __pfx_rcu_cpu_kthread+0x10/0x10
> > > [    0.407966]  smpboot_thread_fn+0x34c/0xa50
> > > [    0.407970]  ? trace_preempt_on+0x54/0x120
> > > [    0.407977]  ? __pfx_smpboot_thread_fn+0x10/0x10
> > > [    0.407982]  kthread+0x40e/0x840
> > > [    0.407990]  ? __pfx_kthread+0x10/0x10
> > > [    0.407994]  ? rt_spin_unlock+0x4e/0xb0
> > > [    0.407997]  ? rt_spin_unlock+0x4e/0xb0
> > > [    0.408000]  ? __pfx_kthread+0x10/0x10
> > > [    0.408006]  ? __pfx_kthread+0x10/0x10
> > > [    0.408011]  ret_from_fork+0x40/0x70
> > > [    0.408013]  ? __pfx_kthread+0x10/0x10
> > > [    0.408018]  ret_from_fork_asm+0x1a/0x30
> > > [    0.408042]  </TASK>
> > >
> > > Currently, triggering an rdp offloaded state change need the
> > > corresponding rdp's CPU goes offline, and at this time the rcuc
> > > kthreads has already in parking state. this means the corresponding
> > > rcuc kthreads can safely read offloaded state of rdp while it's
> > > corresponding cpu is online.
> > >
> > > This commit therefore add rdp->rcu_cpu_kthread_task check for
> > > Preempt-RT kernels.
> > >
> > > Signed-off-by: Zqiang <qiang.zhang1211@...il.com>
> > > ---
> > >  kernel/rcu/tree_plugin.h | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
> > > index 003e549f6514..fe728eded36e 100644
> > > --- a/kernel/rcu/tree_plugin.h
> > > +++ b/kernel/rcu/tree_plugin.h
> > > @@ -31,7 +31,9 @@ static bool rcu_rdp_is_offloaded(struct rcu_data *rdp)
> > >                 lockdep_is_held(&rcu_state.nocb_mutex) ||
> > >                 (!(IS_ENABLED(CONFIG_PREEMPT_COUNT) && preemptible()) &&
> > >                  rdp == this_cpu_ptr(&rcu_data)) ||
> > > -               rcu_current_is_nocb_kthread(rdp)),
> > > +               rcu_current_is_nocb_kthread(rdp) ||
> > > +               (IS_ENABLED(CONFIG_PREEMPT_RT) &&
> > > +                current == rdp->rcu_cpu_kthread_task)),
> >
> > Isn't it safe also on !CONFIG_PREEMPT_RT ?

How about the following?

(current == rdp->rcu_cpu_kthread_task && in_task())

Thanks
Zqiang

>
> For !CONFIG_PREEMPT_RT and  in rcuc kthreads, it's also safe,
> but the following check will passed :
>
> (!(IS_ENABLED(CONFIG_PREEMPT_COUNT) && preemptible()) &&
>           rdp == this_cpu_ptr(&rcu_data))
>
> Thanks
> Zqiang
>
>
> >
> > Thanks.
> >
> > >               "Unsafe read of RCU_NOCB offloaded state"
> > >       );
> > >
> > > --
> > > 2.17.1
> > >
> >
> > --
> > Frederic Weisbecker
> > SUSE Labs