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:     Dai Ngo <dai.ngo@oracle.com>
CC:     Jeff Layton <jlayton@kernel.org>,
        Olga Kornievskaia <kolga@netapp.com>,
        "hdthky0@gmail.com" <hdthky0@gmail.com>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
        "security@kernel.org" <security@kernel.org>
Subject: Re: [PATCH v3 1/1] NFSD: fix use-after-free in __nfs42_ssc_open()
Thread-Topic: [PATCH v3 1/1] NFSD: fix use-after-free in __nfs42_ssc_open()
Thread-Index: AQHZDnweXCJ53drKKku1BZzW7bJsHq5r280A
Date:   Tue, 13 Dec 2022 14:08:34 +0000
Message-ID: <0B0300B5-6E45-4572-810D-6E14610A60B1@oracle.com>
References: <1670885411-10060-1-git-send-email-dai.ngo@oracle.com>
In-Reply-To: <1670885411-10060-1-git-send-email-dai.ngo@oracle.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?mbOdcBdlcVeHTDk92I44krgMJ57fKY413ZPOBUCokCvgDUy6V96tcb/oe6YI?=
 =?us-ascii?Q?hvGUAng/wPK3yE7WWfe6C4pzy2yDCsaeAzcdL52gmgXW911lBsc405BKFunX?=
 =?us-ascii?Q?spJNxhuNRxrvdDByVY/m7fWbNNlixpdoM5rnMoM9DTy6H/zC6cVKMhh9Epkz?=
 =?us-ascii?Q?voD13G6Cy5VXCtFAjrDUuo8EnHYdZwYyIcI/+KQYsvUJYJeE0X7zD4gVRC9J?=
 =?us-ascii?Q?Uj4leteDDGbrngVUDjd7GaUgA6RcgeW+iOcQ7Ph2lX0yvg5WOZZBO3BpJP7T?=
 =?us-ascii?Q?sN6lS+FtM6n/hjDHGsdJLY0WDydaqW0FDy8+2mxskAMRKNR2Bp+PFIvcJyL9?=
 =?us-ascii?Q?xgz+TYAfLJ9HXAy7j/j98Cpcv/nLT095EuiuBq7Mur6r/X+GCIP6mjMpBiKN?=
 =?us-ascii?Q?YkRzKvlPH4ohvnkme0zqF3WI/OPEcdVyoAY/rHSSCf10HOkMhBRRBYiYud+C?=
 =?us-ascii?Q?T0e4Raqo3/oERK4N7tm3buKteQcRp9vcvmFDGX/6tV5hLSYYozqMYuwxTyNU?=
 =?us-ascii?Q?g4gJNCwn5N3iIfQTO7rBV3GA7RTHeDHY0T7tHpNQRWfK26GVCTd53fVvqDh4?=
 =?us-ascii?Q?FbbZ43XQCjPtZUmqnoaofrsQ5vZOy1S/yjiMUIVyYuB80vPfAHZNLP0g4Jy8?=
 =?us-ascii?Q?6PJ7sLfkAf8w2dftDrqejSpO5WM4OudBa3uPtbRGZjbGa7SbKUQvd75QFlyM?=
 =?us-ascii?Q?OsfLakGzfmhzyA8+Q6n2mofPLHUhHbXYhsExtFBNcQFVwXxN7tvwYkNadaaG?=
 =?us-ascii?Q?cjb/7repknGP+kflV5Sj03djQ5uFdmmmOt5kzuEbWiALfZda+gE2qOeIWudL?=
 =?us-ascii?Q?kqhArnZpBWBUgjN9P45tirxDUmT7GrpeJAavCLUgQrWvRopNvn6YL+iNP/Io?=
 =?us-ascii?Q?HbB+rMiwpXgGnJDq5drkM4iQqhTZyXLJ7lOzQhwewnZgQbZsPTFGlL8kBpci?=
 =?us-ascii?Q?ClEBvv/cgD4AavsB82J7b0FN5eUPALiXnnB7geQm5C/8JJq8GdedLVwkG267?=
 =?us-ascii?Q?B313zpc8ziYGSeZWjm7iM/Lwq6kqH1Sna6Y5/mf1VHi72oOJr0+i9GXxQogl?=
 =?us-ascii?Q?dWEafdB3lj4U1w31oZGf3DyVwjsfAqi7/nghZN7XbvILSe9NUa7dpGOEeZ/w?=
 =?us-ascii?Q?fsnli3lzLNU8I3a7LFtDSy/upEwcA/IGBMLeKLlWZmehigxr28w0p0AC/ity?=
 =?us-ascii?Q?LPdTd9rNAkW8ZvfAKcarfrm/63gkTIkHriot24BAgsDSYUMkKeulFsx7qFRx?=
 =?us-ascii?Q?99r+7x48hCSD1KHv/yT2MuAmqcPfgEinrQ0QCV4wjJ2nEKKSxvt059dRIyMG?=
 =?us-ascii?Q?fadWrTqINlfm7qnXyiUwuoJyXb+Uh/IAAYlTds1w9OHH0cjy/qJv43ZYgZ8I?=
 =?us-ascii?Q?OR8YTX7YCwb2BHIPZzkbS+kIbZ0SWc/zuuhweIt5XWkZ5UDLhXFdaNPAGJTy?=
 =?us-ascii?Q?53kMqT3EeTeLXYyqxxdz3409+HzPwAcE97AI1S5CVBEz2rYj165Sad0plCaW?=
 =?us-ascii?Q?zsccQnS1i3AXiia77B3nE8FDkZBJki2nOcRUdY0Zx5galg2tY6VCgapvJonE?=
 =?us-ascii?Q?36IRf5s5OeL3dqD7diRjfcq/607b8O0wDN/WemCOLBsX2ZF0+G7PrzNwF8S/?=
 =?us-ascii?Q?fA=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <0A0DAF6DB7E0A845A8BC245B299FC156@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: CH0PR10MB5131.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 24cf0954-0364-4a5e-e6f4-08dadd1385e8
X-MS-Exchange-CrossTenant-originalarrivaltime: 13 Dec 2022 14:08:34.6471
 (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: bdwgL13csWR6EDn+up7EhuIzrBlKXqhw7bVuVLCqx/8t5SEQfpMXo7gCkMq3oMd+EOQc/y9JKXZ1dpsJVQS+BQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA0PR10MB7181
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.923,Hydra:6.0.545,FMLib:17.11.122.1
 definitions=2022-12-13_03,2022-12-13_01,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 bulkscore=0 mlxlogscore=999 mlxscore=0
 adultscore=0 malwarescore=0 spamscore=0 phishscore=0 suspectscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2210170000
 definitions=main-2212130126
X-Proofpoint-GUID: yzPt5sUdLvYpH0QYfWBkpDj9BiRMjN0o
X-Proofpoint-ORIG-GUID: yzPt5sUdLvYpH0QYfWBkpDj9BiRMjN0o
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 Dec 12, 2022, at 5:50 PM, Dai Ngo <dai.ngo@oracle.com> wrote:
>=20
> Problem caused by source's vfsmount being unmounted but remains
> on the delayed unmount list. This happens when nfs42_ssc_open()
> return errors.
>=20
> Fixed by removing nfsd4_interssc_connect(), leave the vfsmount
> for the laundromat to unmount when idle time expires.
>=20
> We don't need to call nfs_do_sb_deactive when nfs42_ssc_open
> return errors since the file was not opened so nfs_server->active
> was not incremented. Same as in nfsd4_copy, if we fail to
> launch nfsd4_do_async_copy thread then there's no need to
> call nfs_do_sb_deactive
>=20
> Reported-by: Xingyuan Mo <hdthky0@gmail.com>
> Signed-off-by: Dai Ngo <dai.ngo@oracle.com>

LGTM. Thanks for your quick response!

I'll wait for review comments before applying.


> ---
> fs/nfsd/nfs4proc.c | 20 +++++---------------
> 1 file changed, 5 insertions(+), 15 deletions(-)
>=20
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 8beb2bc4c328..b79ee65ae016 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1463,13 +1463,6 @@ nfsd4_interssc_connect(struct nl4_server *nss, str=
uct svc_rqst *rqstp,
> 	return status;
> }
>=20
> -static void
> -nfsd4_interssc_disconnect(struct vfsmount *ss_mnt)
> -{
> -	nfs_do_sb_deactive(ss_mnt->mnt_sb);
> -	mntput(ss_mnt);
> -}
> -
> /*
>  * Verify COPY destination stateid.
>  *
> @@ -1572,11 +1565,6 @@ nfsd4_cleanup_inter_ssc(struct vfsmount *ss_mnt, s=
truct file *filp,
> {
> }
>=20
> -static void
> -nfsd4_interssc_disconnect(struct vfsmount *ss_mnt)
> -{
> -}
> -
> static struct file *nfs42_ssc_open(struct vfsmount *ss_mnt,
> 				   struct nfs_fh *src_fh,
> 				   nfs4_stateid *stateid)
> @@ -1771,7 +1759,7 @@ static int nfsd4_do_async_copy(void *data)
> 			default:
> 				nfserr =3D nfserr_offload_denied;
> 			}
> -			nfsd4_interssc_disconnect(copy->ss_mnt);
> +			/* ss_mnt will be unmounted by the laundromat */
> 			goto do_callback;
> 		}
> 		nfserr =3D nfsd4_do_copy(copy, filp, copy->nf_dst->nf_file,
> @@ -1852,8 +1840,10 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_co=
mpound_state *cstate,
> 	if (async_copy)
> 		cleanup_async_copy(async_copy);
> 	status =3D nfserrno(-ENOMEM);
> -	if (nfsd4_ssc_is_inter(copy))
> -		nfsd4_interssc_disconnect(copy->ss_mnt);
> +	/*
> +	 * source's vfsmount of inter-copy will be unmounted
> +	 * by the laundromat
> +	 */
> 	goto out;
> }
>=20
> --=20
> 2.9.5
>=20

--
Chuck Lever