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:     Olga Kornievskaia <olga.kornievskaia@gmail.com>,
        Jeff Layton <jlayton@kernel.org>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH v2] NFSD: fix use-after-free on source server when doing
 inter-server copy
Thread-Topic: [PATCH v2] NFSD: fix use-after-free on source server when doing
 inter-server copy
Thread-Index: AQHYphKND+E8pLJojUWzV0V0pTRpEa2brg+AgDv1NQCAAABvgA==
Date:   Fri, 9 Sep 2022 18:13:55 +0000
Message-ID: <B0F27CBD-0D84-4293-B976-A120A60FA27E@oracle.com>
References: <1659405154-21910-1-git-send-email-dai.ngo@oracle.com>
 <E146112C-33EE-4143-92F7-7515638301D7@oracle.com>
 <1557e8bb-1739-29a4-a04c-c62732cfc7c4@oracle.com>
In-Reply-To: <1557e8bb-1739-29a4-a04c-c62732cfc7c4@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?jH6O9Oifs4t+GRiKanLc4RxBKRHAY0BK/r3GTWW2rhICl1Rwqfj2XjJNybVm?=
 =?us-ascii?Q?dOTz3AVTJq4pUw006pmXwV8/OaoxwpTEkNVC6AuOoIHJRGEO4kCT8ydPlp9i?=
 =?us-ascii?Q?SysW4GmECdkcAJUlcwvrFSzenU9J+8jz1yRrXCIguowOzw9qFGFH9xTBqhOS?=
 =?us-ascii?Q?IFw4Sz07xJez+Yx/dVIQ9yA3q06GH9zUpSj2Ih76Mw3IyDMmiM0hlBYbkZKd?=
 =?us-ascii?Q?ncehT9hYAtFX+urSCxm3DB7+VJYE131f3ZA8Bz8YvKw38Qof0iIKx84flaLL?=
 =?us-ascii?Q?mWXjpVZlAL1Kx3FVuiPvRAVpRIVwK73X03UPi/6l+WsbzxF1e0EMnzHFb/EF?=
 =?us-ascii?Q?/ZeTPHKdxirl4gvL9a6/rUzB8MtUiDABL6H8LvwUlmhcZTxIyH2oCC7URXE8?=
 =?us-ascii?Q?ivspdnGm1iEEwgVY7FLfVwmIUxRHToE8vsqRDEL2JnfEF0KsDcZThVoIzG9u?=
 =?us-ascii?Q?ZgZJFvD0zwg4OCyHjAZ1mbuonRiGJlUxbV4lHeAZzTl0236lcKVpgfs/Ckle?=
 =?us-ascii?Q?HrIHpYcC5e6+j5z6LUr5L6mM5Lkkv/dRSEgEyrKwVYVEFnL0fRZceqg0yTyF?=
 =?us-ascii?Q?omPpKKW/rTNBbkcOLlJYyKSAkeL7dq4xmDj0uT0gifraTRSeM1cycHn85bQr?=
 =?us-ascii?Q?5+FHvwFKTdC7WYcOTBoZuph16ISyM93WytNAsi2emoSAH984jab+UPEvbNRR?=
 =?us-ascii?Q?8zuL0Qf1+9S0czJjtRjZ/piEyIl7G4+rRCWfMCBCoyMze1krHL8FnlWDb6h2?=
 =?us-ascii?Q?kdN5w/X1h0+owfBYCGAbX25fKxk0zH6STLgQ/eBv05IezWJnw9b010NG4xyy?=
 =?us-ascii?Q?zfioxV3BQ4jtmpqVWblahGRjw6xftOXavKxf8919KyZPxXOLky2XPDlHmnBw?=
 =?us-ascii?Q?qsjsV+ls8CNYdxMHnbYdpfbXW+KzBnTLa8Tr2gi5J2MMozKBl+NlsmQxPGL/?=
 =?us-ascii?Q?XNalfmunMGYlm6Vs6znJaH2TwiZGIoUlsX1ckm997uXGhc4mHZ0Wx7L3EfwS?=
 =?us-ascii?Q?ynJZNarqxAIA+g1Qolw/VMizQeYGR4QVQAgCjeXt9S/GbTpIGHjq7shui52z?=
 =?us-ascii?Q?ozziX6S+FbspMqKs1LpMGlRN3CjOSqyaTW8H6Ll+ae4aSROZxPHFx9w0I4nd?=
 =?us-ascii?Q?S2eEwCtmS1gMcB9rNT58y/tKj7gCZhjLhC1SuTZch5tP/zdT4Y2X5KC70IIZ?=
 =?us-ascii?Q?eia7GIHelH/aoPPFx7VX43I5m/SoEcqSnhKIgx9K8vaMCjvJxtVYeN7BGKik?=
 =?us-ascii?Q?pl+BGPikyiTkIBAd3By43zs2jQqTDI9htqPXwsktJczeL5G6k1gIUMjCIWo1?=
 =?us-ascii?Q?5KcCQurg9/P1UBF0h8NG/CcdLcB1K7eDixPWINceqKdhviOpVpSRkDV+CH9z?=
 =?us-ascii?Q?eGY6Ne6/gog+UYT4J8yYiG7fsPIpOszWE3cGd7sWCCkfBvLL9TlotDDYaA0W?=
 =?us-ascii?Q?veu0TzcP93dYIcBuB30uUQOi1mxCPuDjoyMdfddV0jj9rizd+5FcmAxN54q0?=
 =?us-ascii?Q?QSxYFWpxxEJu6VAT+dhv6GE38SOEOOrL+oTRhHvySIM0yMGgyqEbIcDiUq5B?=
 =?us-ascii?Q?gTEcqw13tyo4CqeHmzUC1Wz9OcIITn1H5RlcYaKzfRev+OsuZBWhScJa0BqD?=
 =?us-ascii?Q?+g=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <85F7C25442644D4881C0FA05FEB30F57@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: 178a8a01-5cc5-45f1-4dd0-08da928f0f32
X-MS-Exchange-CrossTenant-originalarrivaltime: 09 Sep 2022 18:13:55.8967
 (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: 3n97qNcEHkpnA2uuhFRioywouBcWXeVfzf59PFEuGlpnvusTvE8M8UKOqpz4UECbj/vCdPlAJpF/4y5ZjiDBXw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR10MB4782
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-09_08,2022-09-09_01,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 bulkscore=0 suspectscore=0 phishscore=0
 mlxlogscore=999 spamscore=0 adultscore=0 mlxscore=0 malwarescore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2207270000
 definitions=main-2209090065
X-Proofpoint-GUID: WLW6-Z7H0ODZYlvxFL9E7Q65V4JYIwlt
X-Proofpoint-ORIG-GUID: WLW6-Z7H0ODZYlvxFL9E7Q65V4JYIwlt
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,T_SCC_BODY_TEXT_LINE
        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 9, 2022, at 2:12 PM, Dai Ngo <dai.ngo@oracle.com> wrote:
>=20
> Hi Chuck,
>=20
> Is there anything else you want me to do with this patch?

No. It's been pushed to NFSD for-next (6.1).

https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git/commit/?h=3Df=
or-next&id=3Dcdfdfe05ac91618d65d94b28fcbbd324bba3c5bd


> Thanks,
> -Dai
>=20
> On 8/2/22 7:35 AM, Chuck Lever III wrote:
>>=20
>>> On Aug 1, 2022, at 9:52 PM, Dai Ngo <dai.ngo@oracle.com> wrote:
>>>=20
>>> Use-after-free occurred when the laundromat tried to free expired
>>> cpntf_state entry on the s2s_cp_stateids list after inter-server
>>> copy completed. The sc_cp_list that the expired copy state was
>>> inserted on was already freed.
>>>=20
>>> When COPY completes, the Linux client normally sends LOCKU(lock_state x=
),
>>> FREE_STATEID(lock_state x) and CLOSE(open_state y) to the source server=
.
>>> The nfs4_put_stid call from nfsd4_free_stateid cleans up the copy state
>>> from the s2s_cp_stateids list before freeing the lock state's stid.
>>>=20
>>> However, sometimes the CLOSE was sent before the FREE_STATEID request.
>>> When this happens, the nfsd4_close_open_stateid call from nfsd4_close
>>> frees all lock states on its st_locks list without cleaning up the copy
>>> state on the sc_cp_list list. When the time the FREE_STATEID arrives th=
e
>>> server returns BAD_STATEID since the lock state was freed. This causes
>>> the use-after-free error to occur when the laundromat tries to free
>>> the expired cpntf_state.
>>>=20
>>> This patch adds a call to nfs4_free_cpntf_statelist in
>>> nfsd4_close_open_stateid to clean up the copy state before calling
>>> free_ol_stateid_reaplist to free the lock state's stid on the reaplist.
>>>=20
>>> Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
>> Applied to my private tree for testing.
>> I added "Cc: <stable@vger.kernel.org> # 5.6+".
>>=20
>>=20
>>> ---
>>> fs/nfsd/nfs4state.c | 7 +++++++
>>> 1 file changed, 7 insertions(+)
>>>=20
>>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>>> index 9409a0dc1b76..b99c545f93e4 100644
>>> --- a/fs/nfsd/nfs4state.c
>>> +++ b/fs/nfsd/nfs4state.c
>>> @@ -1049,6 +1049,9 @@ static struct nfs4_ol_stateid * nfs4_alloc_open_s=
tateid(struct nfs4_client *clp)
>>>=20
>>> static void nfs4_free_deleg(struct nfs4_stid *stid)
>>> {
>>> +	struct nfs4_ol_stateid *stp =3D openlockstateid(stid);
>>> +
>>> +	WARN_ON(!list_empty(&stp->st_stid.sc_cp_list));
>>> 	kmem_cache_free(deleg_slab, stid);
>>> 	atomic_long_dec(&num_delegations);
>>> }
>>> @@ -1463,6 +1466,7 @@ static void nfs4_free_ol_stateid(struct nfs4_stid=
 *stid)
>>> 	release_all_access(stp);
>>> 	if (stp->st_stateowner)
>>> 		nfs4_put_stateowner(stp->st_stateowner);
>>> +	WARN_ON(!list_empty(&stp->st_stid.sc_cp_list));
>>> 	kmem_cache_free(stateid_slab, stid);
>>> }
>>>=20
>>> @@ -6608,6 +6612,7 @@ static void nfsd4_close_open_stateid(struct nfs4_=
ol_stateid *s)
>>> 	struct nfs4_client *clp =3D s->st_stid.sc_client;
>>> 	bool unhashed;
>>> 	LIST_HEAD(reaplist);
>>> +	struct nfs4_ol_stateid *stp;
>>>=20
>>> 	spin_lock(&clp->cl_lock);
>>> 	unhashed =3D unhash_open_stateid(s, &reaplist);
>>> @@ -6616,6 +6621,8 @@ static void nfsd4_close_open_stateid(struct nfs4_=
ol_stateid *s)
>>> 		if (unhashed)
>>> 			put_ol_stateid_locked(s, &reaplist);
>>> 		spin_unlock(&clp->cl_lock);
>>> +		list_for_each_entry(stp, &reaplist, st_locks)
>>> +			nfs4_free_cpntf_statelist(clp->net, &stp->st_stid);
>>> 		free_ol_stateid_reaplist(&reaplist);
>>> 	} else {
>>> 		spin_unlock(&clp->cl_lock);
>>> --=20
>>> 2.9.5
>>>=20
>> --
>> Chuck Lever
>>=20
>>=20
>>=20

--
Chuck Lever