Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Chuck Lever III <chuck.lever@oracle.com>
To:     Jeff Layton <jlayton@kernel.org>
CC:     Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH 1/3] nfsd: nfsd_do_file_acquire should hold rcu_read_lock
 while getting refs
Thread-Topic: [PATCH 1/3] nfsd: nfsd_do_file_acquire should hold rcu_read_lock
 while getting refs
Thread-Index: AQHY1QEWqnYiB4uxs0SMdlnrLaX8o634WYIA
Date:   Fri, 30 Sep 2022 19:20:54 +0000
Message-ID: <9D4FA4C1-2246-4CAE-BB8A-D152603E3A56@oracle.com>
References: <20220930191550.172087-1-jlayton@kernel.org>
 <20220930191550.172087-2-jlayton@kernel.org>
In-Reply-To: <20220930191550.172087-2-jlayton@kernel.org>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?zTelbHE+kATVZHaTMPzPIobrF5TKjh2x3ineeEeyHkS7iLXcC+koFSFZpZOv?=
 =?us-ascii?Q?OUQsMGygDYAxhFdUM//tdbp7dwhSl+3P1VjcLkunm5IDMW+3GQr25srXAvgg?=
 =?us-ascii?Q?3vzwsaFltUYJN16AxS3Z20LW4oSVnfanATBSREgY/JZV5kGA3EG/MJAeiMyB?=
 =?us-ascii?Q?h+soLChvh3zWF2N1FPnBybSPS97p7A1IaNfpjJQsZ8a4P97sdPE+VxaInODj?=
 =?us-ascii?Q?KrdEofMkfPkTuyvMWihtt7E74Au9jIWGnGqRoSmepk1pTyJGbhC/64ngK7g7?=
 =?us-ascii?Q?A2JMBkPmWnWzEX9DzwQC2r2iHgGZbgOKsdKOHtOSIuHXMWvVRY+EUyaDWvq2?=
 =?us-ascii?Q?VeGdGMr0mAfnATLZOWJ9HiUZZnqxzjDRZxIX4Exfwa5TsEGpPEcOTzNF6M7p?=
 =?us-ascii?Q?qvjI6ppwp95TrxO084lMsCJNLxs8UWCJHdfny4L3PUZG4u+x+rj219aqwxTI?=
 =?us-ascii?Q?vmhXZ6tIYs8UspgsxS9LUXu0yhmWR811erRgMhKBVEKxbw4ml1GqRr5h9Hm9?=
 =?us-ascii?Q?a0GIG7GpI0Ikh5Bw8hJFz5k8W7y6ZDObJUKccFkAwUuflOCpvtVp3eS+i2gA?=
 =?us-ascii?Q?mkmIfS+yBl1qRBq1+gJV1vVcDUZ6MG1dcGzdW17ejtT7sYDOnHJ6buefCPZB?=
 =?us-ascii?Q?qSZ3T2fK3QMDpynt6pfDUmb5qFIklfVPNdsnBhyXYFEBPPzA8g/Dxk677t05?=
 =?us-ascii?Q?ynRekOXfKQmB514SUAZRW13Skuf81lkY1ZDi6Vn3dF1oXsBO3DGPeQQmS/Op?=
 =?us-ascii?Q?lFubCdJhr6BZVzhVmyEt80HYg/u7vsYXCodLBkQHf0TtkFpT1Mhg9KB5dZN9?=
 =?us-ascii?Q?XTG2P9G8p8DBj/fN/xgMOhaAYVgoujkFED9EzD/f9W1d0LXMam5ND52XWlad?=
 =?us-ascii?Q?M4KINhzJ0PYRloDLe1fjbSCRR1jaVVnwZKCvueXemegcHqpda9IMExP3WcUk?=
 =?us-ascii?Q?Oj8bCtxlhuHKOyzNG8xdi0E+IZEJHWHmtxq3dqqs2qxooHUmesZz21SSz+dk?=
 =?us-ascii?Q?fV0GN2pjCnWZp23ankvhXTHSe5ja14Sw6AXSV0dUA/81L37fjT4CA5yc/NlX?=
 =?us-ascii?Q?OOif2QGbztxD7a+EjWEBOVHXfHCMFTpcOa8gQj8L93g6oP8Yr2GoJqgDRfqO?=
 =?us-ascii?Q?wae7dQFr8Vj1+ou6YA0xitzkG13XJl33L4QsEMfQyp52n3kjXyRV1Q9birNF?=
 =?us-ascii?Q?5KnXUrbmY9NScG9G76QH8twJHglah98DptYXUmxiQictQs7FR3FKLO2sz2/E?=
 =?us-ascii?Q?3d/zSstjkJ94ptItnYMTX/IyF+1DXB0Kt/JGjNLIiVzImEvipclItYZH4fna?=
 =?us-ascii?Q?XYOlNFOeoMfMWIpB6wKbkR8sfnXSFqZRnUK9TjJefhQRVbOEtiXG8A/DU2Mb?=
 =?us-ascii?Q?ijUfy/5HSYEUCAmkRWtcxgIPAjgel9O59FuLPI+YwECiRooPffBVKAAq84pg?=
 =?us-ascii?Q?tluVloeuVqteRGGKml/Z+0AFYn+5u0dOaLuuajyvasnTRa/EMdMVWN/MYqbE?=
 =?us-ascii?Q?dc75u2CqOiOZrN89ql8mhnb6Y2czl0DC8ZVcVvL/L4rUWHjOK6xoTtO0yHBH?=
 =?us-ascii?Q?+/IXPI2r0yHreyIz+LM3Srnk80WxT7TpN1TayU5Ls0A5zVzk428WCyV2VsgH?=
 =?us-ascii?Q?MQ=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <A276048A79C8A84E86FD78C6F53AD8C5@namprd10.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BN0PR10MB5128.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: cf4cda09-b819-43d7-cec2-08daa318e55a
X-MS-Exchange-CrossTenant-originalarrivaltime: 30 Sep 2022 19:20:54.8354
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: GepWSR4U95W9vTrDsfqS2eVH1H83mfUspPeZK3kKesc/MRPLU5yJeihoRQC9CaA6Y8LImSUj9z3Tz+rLoZdseQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR10MB4711
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.895,Hydra:6.0.528,FMLib:17.11.122.1
 definitions=2022-09-30_04,2022-09-29_03,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=999 bulkscore=0 mlxscore=0
 suspectscore=0 malwarescore=0 adultscore=0 spamscore=0 phishscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2209130000
 definitions=main-2209300121
X-Proofpoint-GUID: LI6KMICVGjCV0tVdwZwOyvXzOPOd7tWL
X-Proofpoint-ORIG-GUID: LI6KMICVGjCV0tVdwZwOyvXzOPOd7tWL
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,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-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org


> On Sep 30, 2022, at 3:15 PM, Jeff Layton <jlayton@kernel.org> wrote:
>=20
> nfsd_file is RCU-freed, so it's possible that one could be found that's
> in the process of being freed and the memory recycled. Ensure we hold
> the rcu_read_lock while attempting to get a reference on the object.
>=20
> Signed-off-by: Jeff Layton <jlayton@kernel.org>

IIUC, the rcu_read_lock() is held when nfsd_file_obj_cmpfn() is
invoked. So, couldn't we just call nfsd_file_get() on @nf in
there if it returns a match?


> ---
> fs/nfsd/filecache.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>=20
> diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c
> index d5c57360b418..6237715bd23e 100644
> --- a/fs/nfsd/filecache.c
> +++ b/fs/nfsd/filecache.c
> @@ -1077,10 +1077,12 @@ nfsd_file_do_acquire(struct svc_rqst *rqstp, stru=
ct svc_fh *fhp,
>=20
> retry:
> 	/* Avoid allocation if the item is already in cache */
> +	rcu_read_lock();
> 	nf =3D rhashtable_lookup_fast(&nfsd_file_rhash_tbl, &key,
> 				    nfsd_file_rhash_params);
> 	if (nf)
> 		nf =3D nfsd_file_get(nf);
> +	rcu_read_unlock();
> 	if (nf)
> 		goto wait_for_construction;
>=20
> @@ -1090,16 +1092,21 @@ nfsd_file_do_acquire(struct svc_rqst *rqstp, stru=
ct svc_fh *fhp,
> 		goto out_status;
> 	}
>=20
> +	rcu_read_lock();
> 	nf =3D rhashtable_lookup_get_insert_key(&nfsd_file_rhash_tbl,
> 					      &key, &new->nf_rhash,
> 					      nfsd_file_rhash_params);
> 	if (!nf) {
> +		rcu_read_unlock();
> 		nf =3D new;
> 		goto open_file;
> 	}
> -	if (IS_ERR(nf))
> +	if (IS_ERR(nf)) {
> +		rcu_read_unlock();
> 		goto insert_err;
> +	}
> 	nf =3D nfsd_file_get(nf);
> +	rcu_read_unlock();
> 	if (nf =3D=3D NULL) {
> 		nf =3D new;
> 		goto open_file;
> --=20
> 2.37.3
>=20

--
Chuck Lever