Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   "Zhang, Qiang1" <qiang1.zhang@intel.com>
To:     "Zhang, Qiang1" <qiang1.zhang@intel.com>,
        "paulmck@kernel.org" <paulmck@kernel.org>
CC:     "Eric W. Biederman" <ebiederm@xmission.com>,
        syzbot <syzbot+9bb26e7c5e8e4fa7e641@syzkaller.appspotmail.com>,
        "brauner@kernel.org" <brauner@kernel.org>,
        "keescook@chromium.org" <keescook@chromium.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "syzkaller-bugs@googlegroups.com" <syzkaller-bugs@googlegroups.com>
Subject: RE: [syzbot] WARNING in exit_tasks_rcu_finish
Thread-Topic: [syzbot] WARNING in exit_tasks_rcu_finish
Thread-Index: AQHYfnKnPcEG5KPSqUKTUzyVmEvcrK1MN1mLgADFmMCAAHlEgIAAboyggAAbjYCAAALO4IAAAnpA
Date:   Tue, 14 Jun 2022 00:01:57 +0000
Message-ID: <PH0PR11MB588079E5ECB298E2C42C08D3DAAA9@PH0PR11MB5880.namprd11.prod.outlook.com>
References: <000000000000e2fc3f05e141f930@google.com>
 <87bkux1yn4.fsf@email.froward.int.ebiederm.org>
 <20220612202204.GX1790663@paulmck-ThinkPad-P17-Gen-1>
 <PH0PR11MB588027C1A486FCED7885C500DAAB9@PH0PR11MB5880.namprd11.prod.outlook.com>
 <20220613152337.GD1790663@paulmck-ThinkPad-P17-Gen-1>
 <PH0PR11MB5880EB4D4F2B7D91C88BC852DAAB9@PH0PR11MB5880.namprd11.prod.outlook.com>
 <20220613233753.GK1790663@paulmck-ThinkPad-P17-Gen-1>
 <PH0PR11MB58806FA43338A28D72540CB9DAAB9@PH0PR11MB5880.namprd11.prod.outlook.com>
In-Reply-To: <PH0PR11MB58806FA43338A28D72540CB9DAAB9@PH0PR11MB5880.namprd11.prod.outlook.com>
Accept-Language: en-US
Content-Language: en-US
dlp-product: dlpe-windows
dlp-version: 11.6.500.17
dlp-reaction: no-action
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?LUbD/GeIUjXTPOW4hVl5aTC0CzHxwJ7UUxEMJWNy6NnL1Pohp1jt1wZTXuas?=
 =?us-ascii?Q?/LCK4ikXP6h0DtKXabghU1YSIWJ+Boc8mhWQICs5IrYXcw2+8jiHGoYxgFoG?=
 =?us-ascii?Q?tAfJeBlPzpq8YjMswdbG01+cPu7kvZrygIG8bE390jzwoiufder8HB5l2fYf?=
 =?us-ascii?Q?/6jM4zzn/vOOiBIhWqZ2EDgreVHBVN2U6us7H/gCzgc3m6QQgMFTrduzEJmZ?=
 =?us-ascii?Q?EyYcjia3usYRncCn7Z3a9zjnXuoezW/tEvv6XczE96UDRVq4uMIkeXt5I4cp?=
 =?us-ascii?Q?JzrUjtM5jaY6lql9xRwDs6uAJ6QMZZcLKIOS4oIesRBK5GTjf7Crg3uFnNwb?=
 =?us-ascii?Q?UlzwHMsNWTV3/brsE3EBVDPyhoVITVFME1IfVnvchVXGGSj+n4Tp5AAHkRxU?=
 =?us-ascii?Q?FKLlBsBahQJiUa21g1tfEiGzpCQYSzXMsYhBD3g2APgMYN6ogBCz4M2HKVkl?=
 =?us-ascii?Q?XjOW6tEQvN+3mHkevNdulTysOAG/Cf8KSUVq+8088x5LEogUPyKH7q2wKQ7m?=
 =?us-ascii?Q?WMxmYuBq+HtMYdgob1//7nj5Gv9DvVpGci+64DuyCQeSYDwU94YHqaAPOD/I?=
 =?us-ascii?Q?jH7iIl/l5qaAmiHOneJRgMoVbe+IYLw3NRNiHYQdo/w5zBseWpDrV6za+Uj8?=
 =?us-ascii?Q?YpnNDo3+wtVUegEKlGDaWxSZNnk3Mx8mPO+dJzR7Dwy68vYyDvOoovrPFVdX?=
 =?us-ascii?Q?TE2xeJxSQVWynYVjhUgrPcCxK+KZj+b13GSJ7znGpf+jhSw+RkDx1US09OYc?=
 =?us-ascii?Q?kAadlyIaS+oCjmo8YMvTmRKKL3+NOfNiLLWRsDa0HUdWXw+RH7HW2O/rHfYs?=
 =?us-ascii?Q?opegJouhmq1k41D0cQC+lXLlPvG9Y2/pryp6mnDxtFUcOVUAxisgvp8NzKy6?=
 =?us-ascii?Q?3iHlpc2SgoGzoEAPWrusCeobGPdMXWSVFOsoLGsopTOO2EM+v6fqTVrSl1MP?=
 =?us-ascii?Q?QI6LS6ItBAfY82xOtmr/OwgeSLtK+T8MUvt85IrYib9D/hy2tsmUzDT8G0qc?=
 =?us-ascii?Q?i4zavfp/5b/vpbks1LqdLsqYYCt5Wlfqa5agM8mxUXaB/lQmzCGB1fe3gR14?=
 =?us-ascii?Q?/GiDi+FEtwFVgn35sNyX97WkiI5a/zt1gHfP5tpsdJqdRQF9xp6aSjuGxb+X?=
 =?us-ascii?Q?3q7HHpEegjfKlBi8B9pFHaz2Odnb07bgyFlSVuQb6EzsOLtBAeARsHtfJ/sy?=
 =?us-ascii?Q?yj9bpJ4x3ja78lhx29h8jzV0I/qGTrR4b4o3+VhubHdVzeoHPGomXZ/hI968?=
 =?us-ascii?Q?no3nwyCaBDPzQrNuFKxmqYh5YT8dAxrp8A7UfAOEQ6fmCd01nOfgcAuteNai?=
 =?us-ascii?Q?zfDkayQ26YGbvx9X9v9vdLdrAkW3B8BYupVFXfy18xxSdqLoTJJZ4T89DkR/?=
 =?us-ascii?Q?AcsOGmanja9troyc5hXsatl47hYHvfp2InjbtilyoCVvsvAg9RQiFnb76Lju?=
 =?us-ascii?Q?mXIfi25DUUKcnOo40jYeKsRwYQFIWQ0JWtPDqqJFGyk6UtlC12oQnJAMFGo3?=
 =?us-ascii?Q?lLTm9cTEn185aAlnqocN8y9Fp0pANJHWQHRSeo763W6Xn2Hjl+8XCAYTsJXK?=
 =?us-ascii?Q?uNhG1dmt6HPe3dnOn9LSXwydTrrmuE+EC3DjjPXQLiV00MYRC6MmcMNFBlpD?=
 =?us-ascii?Q?hOo40WH2iHlEPFeMjhpaNP7dH2UA56VMn9iaUQ20yy6kD8Ykgmsu3CKxQZQS?=
 =?us-ascii?Q?Js2MqCkHz4ojUUK1FXJxZmZKwfJulgEDY7SSof+KjRjUI5MucFndkqBxGbyU?=
 =?us-ascii?Q?8Vcca0SY6g=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB5880.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 50daa0d9-b694-46ce-f1a6-08da4d991979
X-MS-Exchange-CrossTenant-originalarrivaltime: 14 Jun 2022 00:01:57.8947
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: GeOR8982/STbKMvPyKNvDGDg2aKivYtJum55O9vnalHHGJPbEzxyXnMiOHDcRfYZLlXDLpXUL6KTP/uO3U8wXA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR11MB3418
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-5.6 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,
        T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On Mon, Jun 13, 2022 at 10:26:47PM +0000, Zhang, Qiang1 wrote:
> On Mon, Jun 13, 2022 at 01:55:31PM +0000, Zhang, Qiang1 wrote:
> > > syzbot <syzbot+9bb26e7c5e8e4fa7e641@syzkaller.appspotmail.com> writes=
:
> > >=20
> > > > Hello,
> > > >
> > > > syzbot found the following issue on:
> > > >
> > > > HEAD commit:    6d0c80680317 Add linux-next specific files for 2022=
0610
> > > > git tree:       linux-next
> > > > console output:=20
> > > > https://syzkaller.appspot.com/x/log.txt?x=3D13b52c2ff00000
> > > > kernel config: =20
> > > > https://syzkaller.appspot.com/x/.config?x=3Da30d6e3e814e5931
> > > > dashboard link: https://syzkaller.appspot.com/bug?extid=3D9bb26e7c5=
e8e4fa7e641
> > > > compiler:       gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU =
Binutils for Debian) 2.35.2
> > > >
> > > > Unfortunately, I don't have any reproducer for this issue yet.
> > >=20
> > > I don't understand what is going on in linux-next=20
> > > kernel/rcu/tasks.h looks different than in Linus's tree.  Paul=20
> > > does that mean you have some staged rcu changes?
> >=20
> > >Less than 100 RCU-related patches in -rcu, so not all that bad. =20
> > >;-)
> > >
> > >But yes, this could possibly be an issue in one of those patches.
> >=20
> > > Eric
> > >=20
> > >=20
> > > > IMPORTANT: if you fix the issue, please add the following tag to th=
e commit:
> > > > Reported-by:=20
> > > > syzbot+9bb26e7c5e8e4fa7e641@syzkaller.appspotmail.com
> > > >
> > > > ------------[ cut here ]------------
> > > > WARNING: CPU: 1 PID: 28639 at kernel/rcu/tasks.h:1664=20
> > > > exit_tasks_rcu_finish_trace kernel/rcu/tasks.h:1664 [inline]
> > > > WARNING: CPU: 1 PID: 28639 at kernel/rcu/tasks.h:1664
> > > > exit_tasks_rcu_finish+0x122/0x1b0 kernel/rcu/tasks.h:1006
> >=20
> > >The usual way for this warning to trigger is for these a task to exit =
while in an RCU Tasks Trace read-side critical section:
> > >
> > >	rcu_read_lock_trace();
> > >	do_something_that_causes_task_exit();
> > >
> >=20
> > Hi Paul, wether the following scenarios be considered
> >=20
> > rcu_read_unlock_trace_special
> >    ->if (trs.b.blocked)
> >        ->raw_spin_lock_irqsave_rcu_node
> >        ->list_del_init(&t->trc_blkd_node)
> >        ->WRITE_ONCE(t->trc_reader_special.b.blocked, false)
> >        ->raw_spin_unlock_irqrestore_rcu_node
> >                      ->Inerrrupt
> >                             ->schedule
> >                                 ->rcu_note_context_switch
> >                                     ->rcu_tasks_trace_qs
> >                                          If (___rttq_nesting && !READ_O=
NCE((t)->trc_reader_special.b.blocked)=20
> >                                                         /*___rttq_nesti=
ng  =3D=3D1    &&  (t)->trc_reader_special.b.blocked =3Dfalse*/
> > 				rcu_tasks_trace_qs_blkd(t)    =20
> >    ->WRITE_ONCE(t->trc_reader_nesting, 0)
> >     .......
> >    -> exit_tasks_rcu_finish
> >=20
> > Whether the following patch can fix it, or what am I missing?
> > Any thoughts?
> >=20
> > diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h index
> > f1209ce621c5..c607e4c914d3 100644
> > --- a/kernel/rcu/tasks.h
> > +++ b/kernel/rcu/tasks.h
> > @@ -1247,6 +1247,7 @@ void rcu_read_unlock_trace_special(struct task_st=
ruct *t)
> >         struct rcu_tasks_percpu *rtpcp;
> >         union rcu_special trs;
> >=20
> > +       WRITE_ONCE(t->trc_reader_nesting, 0);
> >         // Open-coded full-word version of rcu_ld_need_qs().
> >         smp_mb(); // Enforce full grace-period ordering.
> >         trs =3D smp_load_acquire(&t->trc_reader_special);
> > @@ -1267,7 +1268,6 @@ void rcu_read_unlock_trace_special(struct task_st=
ruct *t)
> >                 WRITE_ONCE(t->trc_reader_special.b.blocked, false);
> >                 raw_spin_unlock_irqrestore_rcu_node(rtpcp, flags);
> >         }
> > -       WRITE_ONCE(t->trc_reader_nesting, 0);
> >  }
> >  EXPORT_SYMBOL_GPL(rcu_read_unlock_trace_special);
>=20
> >Thank you for looking into this!
> >
> >You do have what I believe to be the correct failure scenario, but the a=
bove fix would break nested RCU Tasks Trace read-side critical sections.
>=20
> Hi Paul
>=20
> Break nested RCU Tasks Trace read-side critical sections? =20
> Does it mean the following?
>=20
> rcu_read_unlock_trace
>     -> WRITE_ONCE(t->trc_reader_nesting, INT_MIN);
>          /* t->trc_reader_special.s  =3D=3D 0*/
>     -> if (likely(!READ_ONCE(t->trc_reader_special.s)) || nesting)
>           -> Interrupt
>               -> schedule
>                  -> rcu_note_context_switch
>                      -> rcu_tasks_trace_qs
>                               /*___rttq_nesting  =3D=3D INT_MIN    &&  (t=
)->trc_reader_special.b.blocked =3D=3D false*/
>                           ->rcu_tasks_trace_qs_blkd(t)    =20
>              /*nesting =3D=3D 0*/
>          -> WRITE_ONCE(t->trc_reader_nesting, nesting);
>          -> return;

 Directly return.

>  .........
>  exit_tasks_rcu_finish
>      trigger Warnings
>=20

My change ignores this, so I described the above scenario, the rcu_read_unl=
ock_trace_special() is not be called.

Thanks
Zqiang

> Or where am I misunderstanding?

>I suspect that you do in fact understand it.  Let's walk through the failu=
re scenario again and see.
>
>If that READ_ONCE(t->trc_reader_special.s) return zero as you suggest,=20
>and then the interrupt and schedule happen as you suggest, then
>rcu_tasks_trace_qs_blkd() will see a non-zero ->trc_reader_nesting and=20
>a zero .b.blocked.  It queues the task, but rcu_read_unlock_trace()=20
>won't check again.  It will set ->trc_reader_nesting to zero and=20
>continue.  As you noted, if the task exits in that state, then
>exit_tasks_rcu_finish_trace() will trigger its WARN_ON_ONCE().

I understand the problem you describe, Just like the call flow I described =
above,
rcu_read_unlock_trace_special() may not be called.

Thanks
Zqiang
=20
>
>Your change won't affect this because rcu_read_unlock_trace_special() neve=
r gets called.
>
>Hence the approach in the patch below.
>
>Do you see any failure modes given the below patch?

> Thanks
> Zqiang
>=20
> >
> >But would you be willing to try out the patch shown below?
>=20
> I will try to test it.

>Thank you very much!
>
>						Thanx, Paul

> >---------------------------------------------------------------------
> >---
>=20
> diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h index=20
> 08059d8d4f5a7..937a58b3266bf 100644
> --- a/include/linux/rcupdate.h
> +++ b/include/linux/rcupdate.h
> @@ -184,7 +184,7 @@ void rcu_tasks_trace_qs_blkd(struct task_struct *t);
>  		if (likely(!READ_ONCE((t)->trc_reader_special.b.need_qs)) &&	\
>  		    likely(!___rttq_nesting)) {					\
>  			rcu_trc_cmpxchg_need_qs((t), 0,	TRC_NEED_QS_CHECKED);	\
> -		} else if (___rttq_nesting &&					\
> +		} else if (___rttq_nesting && ___rttq_nesting !=3D INT_MIN &&	\
>  			   !READ_ONCE((t)->trc_reader_special.b.blocked)) {	\
>  			rcu_tasks_trace_qs_blkd(t);				\
>  		}								\
> diff --git a/include/linux/rcupdate_trace.h=20
> b/include/linux/rcupdate_trace.h index 6f9c358173989..9bc8cbb33340b
> 100644
> --- a/include/linux/rcupdate_trace.h
> +++ b/include/linux/rcupdate_trace.h
> @@ -75,7 +75,7 @@ static inline void rcu_read_unlock_trace(void)
>  	nesting =3D READ_ONCE(t->trc_reader_nesting) - 1;
>  	barrier(); // Critical section before disabling.
>  	// Disable IPI-based setting of .need_qs.
> -	WRITE_ONCE(t->trc_reader_nesting, INT_MIN);
> +	WRITE_ONCE(t->trc_reader_nesting, INT_MIN + nesting);
>  	if (likely(!READ_ONCE(t->trc_reader_special.s)) || nesting) {
>  		WRITE_ONCE(t->trc_reader_nesting, nesting);
>  		return;  // We assume shallow reader nesting.