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:     Xingyuan Mo <hdthky0@gmail.com>
CC:     Dai Ngo <dai.ngo@oracle.com>, Jeff Layton <jlayton@kernel.org>,
        Olga Kornievskaia <kolga@netapp.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: AQHZDnweXCJ53drKKku1BZzW7bJsHq5rK+aAgACuE4A=
Date:   Tue, 13 Dec 2022 14:02:01 +0000
Message-ID: <9CCA3C67-3A27-42BA-93D1-7665A051F485@oracle.com>
References: <1670885411-10060-1-git-send-email-dai.ngo@oracle.com>
 <CALV6CNPysKmTDmeZds61eKrtmA-yGbj1pQKvxOtfkpF3P5ankw@mail.gmail.com>
In-Reply-To: <CALV6CNPysKmTDmeZds61eKrtmA-yGbj1pQKvxOtfkpF3P5ankw@mail.gmail.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?5eQ1flPDJYlxWy6YR6J9zKe7+/vWdZuYzkHLnAHCDKKhGxsUl7JNTAMETuAX?=
 =?us-ascii?Q?ItqUjz2GqvSNYVUD8o/FntIyqu/iomMgwj+7ZKVSqBjdurdmCDTJTpXeHMtP?=
 =?us-ascii?Q?L9g9u2scrJXUy/sejXgLca84UJdxAoJ2i/QomLDWd2mloJcVoKw/QRF9pPD2?=
 =?us-ascii?Q?yvg1KGGTqmzv2bJsdiPKMXK4jDdywU3kSd184VMLam7nt7JKkZTUdhm/Lt1P?=
 =?us-ascii?Q?jtyL3Gjm/W+GjnGoZiEy7KdaBYFRCBE72Zep7Jr7VoOIEVCm2oLRKJgj2WVH?=
 =?us-ascii?Q?7qMdo2fCUmEACtRNtTJEDyXSfDCz8DSQCrc1fTLzXr1hustKJkyTiBoou0Bq?=
 =?us-ascii?Q?hCjiEJDrrIGacBx7jcZL0wmQMbJCpzZDfDsNNcGdzD0sQffaQDOECA89bU+g?=
 =?us-ascii?Q?fl75IXTkJzl21Bj0qerhdubvb2RIZPZYLvKFi3FxIyDF6PkF1Z3wKBtOC7BH?=
 =?us-ascii?Q?/InuPlycRHNLwI+SclBBDmGXPrBuSHHj2A8kymUbpuwXK5wRuiZ18ARGQmEu?=
 =?us-ascii?Q?K29pMnJR16mdytShH8ZztxPKjOm46cvv5hH3ffMezchtAjjOOxTwakx8Y61B?=
 =?us-ascii?Q?HZhREd8piqW8jFN0OmbmweGsBE7v4UfbxAzvKSUzIwd4KbcJ+H6mPBf9mb1n?=
 =?us-ascii?Q?m//o8DL6YVYBTra7oRgBXfTkMLNWTnTVsmxsqetR8cqyE0cP7JH9Pb6u7Nf9?=
 =?us-ascii?Q?ziAZtJV2dtXbMKXLVUxywgKCU1wujrCY25/VkMxx+xdz/NbJbdGAZQR0uxmd?=
 =?us-ascii?Q?VAMDPicEDrueeJBGXwWVJvaHMe7+ZRM/MZML6GP3e9JyjnQ/+kKSWKFZsqwn?=
 =?us-ascii?Q?+tuQD7z3jzXUPo+rFFgISqvDVJG+lkDDToIcBCLJtAKWV6LYxpPhNM+FCJ9r?=
 =?us-ascii?Q?ecl56mSDTV0AY+uCqRyTZzH+hJDHhP9XXcsgxfhnU8O5DxspGyrLUky5odbj?=
 =?us-ascii?Q?bSrdfGWkArx2eqxt9sVTtwpLQKnumCsS+TGmPtsmg4e8W4gECZFSfosYhtMh?=
 =?us-ascii?Q?tx/2Zg/BoGPzVBQ4NvLcBHVZ/FIFKCvLfOyGMQnt2BkJ1tr1CVfQDwSce2Hr?=
 =?us-ascii?Q?+TKVsF+VZVsUCWX4EVy54J3mtK7YtXB8xsiKC6r0Y2ffSEXt3Fir7knxsEIp?=
 =?us-ascii?Q?6DU2GIkdg6k8nmD9MuetIe+bSX12D5NYmbajXGesoxBcy0QzxOWeDo1eApt6?=
 =?us-ascii?Q?XL5aaC6hfGKq3DxmrMFoMHprHK/jJkt7477iQvQKqcJOHhQxulrUOo5SQQOE?=
 =?us-ascii?Q?d4fryhv6T7Hka6stko3lGbTAV04rf9sF9A8xWUXv05qt6uqWb/JVKzjg2xds?=
 =?us-ascii?Q?5xN4iBYGGI0vooHqXQl/4RqnMXi49ayDSuVRjeqOw7Jg5hYazdJKFlSfau6U?=
 =?us-ascii?Q?YMuqaXa5A0j8AgxSz6MlAzspA5Tl1uyk1KW67bs4xbTgG71ZVVTk0ivqF0aq?=
 =?us-ascii?Q?8b9gGmAGEsM3R7SUGz9COd9/WXGbkk8rSV5uBoInUTaves8bpHfCgZAPl5oY?=
 =?us-ascii?Q?rYkKbaSL5XbhWSb3c3MmDauLseBV4la+R+Sn7Ia2b7pg0M0kL6Cl1w4/Z2cC?=
 =?us-ascii?Q?ZJTNp/S0NvmzduhdPvBX0b8hqPzv8G4EXWH/M3wlKzWpGszkFIiNjfUdWdnD?=
 =?us-ascii?Q?BQ=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <5BCFEB0D84C02A41A304FC09C60C75ED@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: 1a921cad-9fbd-4c2c-2449-08dadd129bcc
X-MS-Exchange-CrossTenant-originalarrivaltime: 13 Dec 2022 14:02:01.8874
 (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: vhvrkC5XRglz15C+5ZUpd6ZkPAYZW7MjS9EMFxxK8VoP/EQcBmzCe32mBYju/OmTpeNm5WcE7j30gKDFxOqZCw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR10MB5781
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 spamscore=0 phishscore=0 bulkscore=0
 malwarescore=0 mlxlogscore=999 mlxscore=0 adultscore=0 suspectscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2210170000
 definitions=main-2212130125
X-Proofpoint-ORIG-GUID: Y43I8-djomg9Uokt62mU31cIP5Kc2V-T
X-Proofpoint-GUID: Y43I8-djomg9Uokt62mU31cIP5Kc2V-T
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 10:38 PM, Xingyuan Mo <hdthky0@gmail.com> wrote:
>=20
> On Tue, Dec 13, 2022 at 6:50 AM 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>
>> ---
>> 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, st=
ruct 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, =
struct 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_fil=
e,
>> @@ -1852,8 +1840,10 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_c=
ompound_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
>> --
>> 2.9.5
>>=20
>=20
> My test results show that this patch can fix the problem.

May I add "Tested-by: Xingyuan Mo <hdthky0@gmail.com>" to Dai's patch ?


--
Chuck Lever