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;
Message-ID: <bdb9f9f5-5c13-44c1-8979-89a7b77afc21@oracle.com>
Date:   Mon, 12 Dec 2022 23:00:10 -0800
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0)
 Gecko/20100101 Thunderbird/102.5.1
Subject: Re: [PATCH v3 1/1] NFSD: fix use-after-free in __nfs42_ssc_open()
Content-Language: en-US
To:     Xingyuan Mo <hdthky0@gmail.com>
Cc:     chuck.lever@oracle.com, jlayton@kernel.org, kolga@netapp.com,
        linux-nfs@vger.kernel.org, security@kernel.org
References: <1670885411-10060-1-git-send-email-dai.ngo@oracle.com>
 <CALV6CNPysKmTDmeZds61eKrtmA-yGbj1pQKvxOtfkpF3P5ankw@mail.gmail.com>
From:   dai.ngo@oracle.com
In-Reply-To: <CALV6CNPysKmTDmeZds61eKrtmA-yGbj1pQKvxOtfkpF3P5ankw@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?ZVZOM21uRjBoT0ZSLzJrYitqQTAvQ25NME5hbXdlc2wwU2dyZEswMG5mMkpK?=
 =?utf-8?B?SXF4VFhWaVFET1NnaGh5YkJmWkQ0ZG0yc3FxRGwzc1A3eStuOHdxaEZueTU0?=
 =?utf-8?B?eFFXZENJdE5JMlpJQ1dJM3pxSHFJQ2IydDh4REFTMjRhV3d4NU1WZzRRYkJW?=
 =?utf-8?B?QUF5b3FqZTM3MXB3MURFbFZES2F3Z0xySlE3YlZIdE9XVUNsZ256TUdZVnpr?=
 =?utf-8?B?K3NuZ1NaQVFBU2F6aXBMUEErc0hwRlJXbkkrUHZvSHV3bHByMWs4dE1XWkMr?=
 =?utf-8?B?SXA4Rzhvc0czUmUzSVAxZzZJWCtzK1FzUXZqcWMwK1lrS2RDNGNXS3FSbnUw?=
 =?utf-8?B?dlRNbFJuOXRqMHpzMGtUci82QTg5dnkyc2tHZ2EzMjBmTUd3Q25hTnhQV1ls?=
 =?utf-8?B?SmJvVE5ZWDVoS1FhUHlPNis0MXFXOVpGVnNYQWljN0FMTy9kZ2tSdVhPY1Yx?=
 =?utf-8?B?N2l6cDR0OVoyZW5VeDZ2SEdYcjBGOTM3SWMzREMzN3pKYXFVckp5MXpVL2Vz?=
 =?utf-8?B?MExPVWtRaGJWRnB6dXZWd0VLNExhZTJJV2hqdERrMU1keEdkeE1qQVFqSHUr?=
 =?utf-8?B?NTlJdVB0K3hnRUMwaFVjRTFzZlpnVmlvbFdhR3AzRGMyNlVjNnJPMm5BTVJF?=
 =?utf-8?B?MElJVjJ6bDV6eitzZC9iUTk1V09uQVg1VEtCOW51VlZlVGhBTE5Tc3JaNkNF?=
 =?utf-8?B?Z1pkUFJmWDlaTzE2N3VVR2Z6S2p4U3dWTjI1b0FrVW82SlI5VWpOWExyQ0t3?=
 =?utf-8?B?RjBxZXZlWWI2MjNsSnRUNlVRV0I5RUNBUXAzdi8xMThDa0Y1cmVWOVdMbnR0?=
 =?utf-8?B?U3AyenRYaXYxaGx1NWU4Nkxob3llcytCZ29rbitXalFUWHZ3UjRqemFmYjds?=
 =?utf-8?B?TWdncmk1QVhmczV4enpXdWd5ajJRMVVVU0kySkFiT1JyenRiNXdWbm5IcFFM?=
 =?utf-8?B?K0pqRXQ2THkzWEdlaGw5SFIwSTdxZTgyamxteTFKaXhzK3QyQUp4T2l2S1N4?=
 =?utf-8?B?RDN3Z2NRVFptSUkxd0toZzhycDRKc2hDOXZmcmRmMjBvN2tKbFhyOCtvQkpn?=
 =?utf-8?B?U0RKeEZoTDMwSXlEdFFDelN5NUlEVW9wQlJjVnFJdjloMmttaXZacjBJdHQx?=
 =?utf-8?B?RFc1NGd5bXJ2ay82aS84NHJXenFlZDREMFQ1R0M5NXJ6dkhWZnZNSlZDMmpP?=
 =?utf-8?B?NnBhYTkrdDFlN2lzOStpWU9OR0Zxc3ZnWXUvSWl4NVN6QVMxREE0cDByUG1I?=
 =?utf-8?B?Yk51NmFpQTFac2krZDFUS0RQQWlNR2o0NVNScGZidWlTTUlNWmJqekRlWDYx?=
 =?utf-8?B?RVo3WDJYRW9ZTHZZRE9rdmZ3UE90bmx1MEx2T3gyTWtHcHJqdVNVczJXbE02?=
 =?utf-8?B?L0QrZGNGMVV3M2N4c0JhQUgzSE1DeEQ5ZmZ3eFcyWXZFcjJqdlpzQmRJYzc2?=
 =?utf-8?B?T2FRQmxYRFpZNi9qSzROczZBa3lOMTlLOXhjNE1YWEsvSWxQaHk2SDJmZkxE?=
 =?utf-8?B?djdmaGpHNTdVUTdGaExhb0dvaDZpb1pXc1dsMWlxclUwcGdkY3A0U2k2N0s5?=
 =?utf-8?B?cFZ0SWtpUTlLRHl5V0dyOTBZTjBnNERHWEp3SGsyWS9VUFBndzRkbDJBNTNE?=
 =?utf-8?B?R01SSDUwTTkzNlJlVkhETUN2VFJpMS9CZ1ZrRXhZNlljUjhyeWxlMVIwUytC?=
 =?utf-8?B?RGRDQkp2a2RxTTM2ZDFBUmtWS0lCUzhNaExPK3hSd0p4cTl2b0ZlYTRQOE1G?=
 =?utf-8?B?MmZYSkJDcHppL0ZBYXFSd0tyQmxRMUJjMVR6RC9OQ3VLRmhZd3p3dlU5NUJv?=
 =?utf-8?B?bWhONmdOV1dMWnVKcS9PSE83ZTF4NEVmaVNsem1SRkl6WE1aRmRyRlg4T2ZT?=
 =?utf-8?B?bDR1a0Q1YTdsVnR3aDRuVUZtaEM0azlvKzN6S0ZuV0lrQnQvZFA0L1Vnc2Jy?=
 =?utf-8?B?YThUSW5SOGYwdCt2bTVrcGFOWU9tTmhIci9Ic0xpQlZPTWRscDZIMjJieWd4?=
 =?utf-8?B?bDNwNmJIS0ZnTC9lWHB3MFJVa1poZUN4cGJ3RnNMbDBOWkRaZmtFYWY2b21n?=
 =?utf-8?B?cW5SMGFmWGlZK0RiNnhmazBTQkt2eEpDOEwzb0h3bXV1NWJFR29vZnk3MjVL?=
 =?utf-8?Q?yURuwJG+mMI3xyvcXv3cLj0H+?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: f861fb9a-8673-4e97-7ea2-08dadcd7ae34
X-MS-Exchange-CrossTenant-AuthSource: BY5PR10MB4257.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Dec 2022 07:00:12.6976
 (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: ge0zu3BsuRs/IaMn/ZNp3dVcFnycVFHGupfykyhDBlh32MmQIVrnc2jF5n6GqPA9hSkDBzZwTtk/9uUnods/yQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR10MB4875
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-12_02,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 adultscore=0 suspectscore=0 mlxscore=0
 mlxlogscore=999 malwarescore=0 spamscore=0 phishscore=0 bulkscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2210170000
 definitions=main-2212130063
X-Proofpoint-ORIG-GUID: Pl-XuP1V_dvyMKV9VBn8ViXis4k9aYrs
X-Proofpoint-GUID: Pl-XuP1V_dvyMKV9VBn8ViXis4k9aYrs
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,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 12/12/22 7:38 PM, Xingyuan Mo wrote:
> On Tue, Dec 13, 2022 at 6:50 AM Dai Ngo <dai.ngo@oracle.com> wrote:
>> Problem caused by source's vfsmount being unmounted but remains
>> on the delayed unmount list. This happens when nfs42_ssc_open()
>> return errors.
>>
>> Fixed by removing nfsd4_interssc_connect(), leave the vfsmount
>> for the laundromat to unmount when idle time expires.
>>
>> 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
>>
>> 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(-)
>>
>> 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, struct svc_rqst *rqstp,
>>          return status;
>>   }
>>
>> -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,
>>   {
>>   }
>>
>> -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 = nfserr_offload_denied;
>>                          }
>> -                       nfsd4_interssc_disconnect(copy->ss_mnt);
>> +                       /* ss_mnt will be unmounted by the laundromat */
>>                          goto do_callback;
>>                  }
>>                  nfserr = nfsd4_do_copy(copy, filp, copy->nf_dst->nf_file,
>> @@ -1852,8 +1840,10 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>>          if (async_copy)
>>                  cleanup_async_copy(async_copy);
>>          status = 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;
>>   }
>>
>> --
>> 2.9.5
>>
> My test results show that this patch can fix the problem.

Thank you Xingyuan for testing the patch.

-Dai

>
> Regards,
> Xingyuan Mo