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>,
        "frederic@kernel.org" <frederic@kernel.org>,
        "quic_neeraju@quicinc.com" <quic_neeraju@quicinc.com>,
        "joel@joelfernandes.org" <joel@joelfernandes.org>
CC:     "rcu@vger.kernel.org" <rcu@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH] rcu: Safe access to rcu_node structure's->exp_tasks
Thread-Topic: [PATCH] rcu: Safe access to rcu_node structure's->exp_tasks
Thread-Index: AQHZF1d1vL7vqzT7Rk6Qh2aIcidsQa6QyRzw
Date:   Fri, 6 Jan 2023 03:10:12 +0000
Message-ID: <PH0PR11MB58801E84B4601F207B02377FDAFB9@PH0PR11MB5880.namprd11.prod.outlook.com>
References: <20221224052553.263199-1-qiang1.zhang@intel.com>
In-Reply-To: <20221224052553.263199-1-qiang1.zhang@intel.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?6ECiOgJhJzew02c51fG9emV/TR2kRSDZcj0iS1x3U6CKJNeDuKOOe37Ti/jT?=
 =?us-ascii?Q?UowozRT4aakoqpCK3oVK2Bi9Hi+5RY8YLniLPhqhevOqQzyMVqePbDJEZINb?=
 =?us-ascii?Q?DtF75UTLHDOHKO7ScVgpqrR2U/xwL9B7jOXdU9yIY28+FSA4Dmfz1qPh6m4Q?=
 =?us-ascii?Q?d71RcUnjicXtatRdd7iz30ZWmkiEACtqbUhcNl2cJ1yc0wzXO2QNDWyTagMd?=
 =?us-ascii?Q?KjGVpl4pFS7EqJUnTIXtbpwDkXhmPbbjwX5c2t7jdvhP+KQe8wG20FcH0mlK?=
 =?us-ascii?Q?jMf7hF4KGWj63e9qfbAaoAXBA3gTz6RwTHtDDk9Y3g/qdNLaRFL+87IUsG09?=
 =?us-ascii?Q?b8NK8PKjnFwCXLQ80SvqZEBjLhKiHyz0bQEsgCEgjiknn1mLdll7/De3nBNQ?=
 =?us-ascii?Q?L95vawD+UBQBsjHNeFuPo8BhtRemn45TLCyYTshYXSN+9GK+UqJ50P+pGsDv?=
 =?us-ascii?Q?bbrC+sTOj3Tb7hZhsqkPTCfA7kxHb2+NG4T9qSw67L6m8HhViad4dNdUlBkE?=
 =?us-ascii?Q?cF6C+3mFJwJnWp9aGo0QAnFkWFx4W2C+/hbP46BoT/4RYm/716pA9G+9VJO4?=
 =?us-ascii?Q?N4BYrn87OBTXmGCMK/RovjHfBtn3FfA0QAgdOsyppK9XB4D5ycYE8CPEmFn+?=
 =?us-ascii?Q?lTeLEeOWz0Cs9D9WBcP2uPhE6lHFLnUm9ySBo/MWMsF3DutI68AW03j24yut?=
 =?us-ascii?Q?yBnxiNTNWdPmpl9FT/Z2rRkqNdLGj09ejpd1S+oCg77SiZ4foP7s76KjSwqU?=
 =?us-ascii?Q?hXLTmaw4mqHkQuqD32P2Emy03Q/ow1iR3T9uYTlOrcmhHKc2WH0p++NNmUAL?=
 =?us-ascii?Q?y18mNUmYWyUnSjLkXpuRoU/2xhPSS09GMJMhW7iRhXZiRKSaeVXyy4aThlEn?=
 =?us-ascii?Q?PGTfeTNhBgb78Kh9Mq1E/JOzyl1mZBLRxZj8eYKbNze4c1uwEyEgkugagjBk?=
 =?us-ascii?Q?24ItP2dxff5xmIlDWReHymrecPGoUrUQ0DN4cTC/z+5ORicGBtsZlJo2kaRc?=
 =?us-ascii?Q?LG1R090MqUdfq4EirXzNRud8wmbUCvxKobuMOjAnjRj4TaZgMgFff3g14jl1?=
 =?us-ascii?Q?7QujaGwI/VacO606gGwwkEgFmTZt3gG22aRVac4FH9ooxErq3T/30jrgNMsT?=
 =?us-ascii?Q?ScGprRkEdP3rRmUw4rMT3QgOcx3tXaiDnuJvCPWuAlAqKNEMNVKICEdxo/ud?=
 =?us-ascii?Q?i+6dp6DQJz2gkg9JcX0rR+nZOD7KCHf0ZTcaRFpuUEIIUhZ5oQz1fGXIR7pO?=
 =?us-ascii?Q?8aCZmOY3TtK7SY3xaMzrYO/Btik/geiVulZrvpYtuMHY0MdvFSvpfSpPD2ht?=
 =?us-ascii?Q?80dOfUE2hHqytlEsxHQHti7OzPNOHEDvl5Y7X6WtpWeWr0qbpYAwJT/wDwI9?=
 =?us-ascii?Q?R1f1ZBO4QICu+4yDC9T6I6k8/+eNGF6tkYvWKoy2a10ZqDqoNya7iaHD8y55?=
 =?us-ascii?Q?3cPCuEaKL5q371bgxUjXNhTjAAkCVEjQWyQ16jLe66yTjnn4f4zQrixIVGY0?=
 =?us-ascii?Q?c1Gs0wWzzwB+bj4Sv6TORcXw4N/V9/v1tf3tP2+C/G4Nb99KO0X4sHLpQOOS?=
 =?us-ascii?Q?oP7s8bk4spd0wBThYRNMfU40Foscp/DPsge8fG3w?=
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: abc0313b-cc16-4a99-8449-08daef9386b1
X-MS-Exchange-CrossTenant-originalarrivaltime: 06 Jan 2023 03:10:12.5046
 (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: z+G9095mjd5DCA9jhNH13Qf9xrl/uvC5/YsESgKqw3RDUOMZlmCXLcbYC23GjEnBlbSOmWiyT9Qj/+VdgvpWaw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN9PR11MB5560
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 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
        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

>For kernels built with CONFIG_PREEMPT_RCU=3Dy, the following scenario
>can result system oops.
>
>           CPU1                                           CPU2
>rcu_preempt_deferred_qs_irqrestore                rcu_print_task_exp_stall
>  if (special.b.blocked)                            READ_ONCE(rnp->exp_tas=
ks) !=3D NULL
>    raw_spin_lock_rcu_node
>    np =3D rcu_next_node_entry(t, rnp)
>    if (&t->rcu_node_entry =3D=3D rnp->exp_tasks)
>      WRITE_ONCE(rnp->exp_tasks, np)
>      ....
>      raw_spin_unlock_irqrestore_rcu_node
>                                                    raw_spin_lock_irqsave_=
rcu_node
>                                                    t =3D list_entry(rnp->=
exp_tasks->prev,
>                                                        struct task_struct=
, rcu_node_entry)
>                                                    (if rnp->exp_tasks is =
NULL
>                                                       will trigger oops)
>
>This problem is that CPU2 accesses rcu_node structure's->exp_tasks
>without holding the rcu_node structure's ->lock and CPU2 did not
>observe CPU1's change to rcu_node structure's->exp_tasks in time,
>if rcu_node structure's->exp_tasks is set null pointer by CPU1, after
>that CPU2 accesses members of rcu_node structure's->exp_tasks will
>trigger oops.
>
>This commit therefore allows rcu_node structure's->exp_tasks to be
>accessed while holding rcu_node structure's ->lock.
>
>Signed-off-by: Zqiang <qiang1.zhang@intel.com>
>


Hi Paul,  some suggestions for this modification?

Thanks
Zqiang


>---
> kernel/rcu/tree_exp.h | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
>diff --git a/kernel/rcu/tree_exp.h b/kernel/rcu/tree_exp.h
>index 7cc4856da081..902e7c8709c7 100644
>--- a/kernel/rcu/tree_exp.h
>+++ b/kernel/rcu/tree_exp.h
>@@ -803,9 +803,11 @@ static int rcu_print_task_exp_stall(struct rcu_node *=
rnp)
> 	int ndetected =3D 0;
> 	struct task_struct *t;
>=20
>-	if (!READ_ONCE(rnp->exp_tasks))
>-		return 0;
> 	raw_spin_lock_irqsave_rcu_node(rnp, flags);
>+	if (!READ_ONCE(rnp->exp_tasks)) {
>+		raw_spin_unlock_irqrestore_rcu_node(rnp, flags);
>+		return 0;
>+	}
> 	t =3D list_entry(rnp->exp_tasks->prev,
> 		       struct task_struct, rcu_node_entry);
> 	list_for_each_entry_continue(t, &rnp->blkd_tasks, rcu_node_entry) {
>--=20
>2.25.1
>