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>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH] NFSD: fix use-after-free on source server when doing
 inter-server copy
Thread-Topic: [PATCH] NFSD: fix use-after-free on source server when doing
 inter-server copy
Thread-Index: AQHYpRrq3VSBDvRXmU2iM7U3Z61RMa2aPX+A
Date:   Mon, 1 Aug 2022 16:29:21 +0000
Message-ID: <37585D43-78F3-4132-8ADF-D11BD11DDCD4@oracle.com>
References: <1659298792-5735-1-git-send-email-dai.ngo@oracle.com>
In-Reply-To: <1659298792-5735-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?K0k0bwA9zA1z04K0pUehrLiE7jHDm2tJrqxRqNERgYpeXtitTwhTFejj/YAV?=
 =?us-ascii?Q?s43XvJSWgoP3DdOeWw+khpAP0uDDPCt7FZPkU8gJJ7i5r4MlGCM224su/PNP?=
 =?us-ascii?Q?+M3vDxWqXxrV+9dhNjGX+Wk48lq1aoC7uOgV7Org77LY1K5qEgngrgeO6S91?=
 =?us-ascii?Q?4ihMDvCfNf5axtqkAKdWYWiBbJsiFJTJqnnFO5+U1c7hjFVy91Hqb95HqHNw?=
 =?us-ascii?Q?IcQ67jIhRG0hLYn1Has92KLD6fPfrEruy0+QQcLTLyjSjW+pp7Kxjic45kNE?=
 =?us-ascii?Q?x/qG3dsE6EZq1DKYRF/FwWlPObRCOG0ZxOXA7f02gATrYo4Kc91ugxXyXeJT?=
 =?us-ascii?Q?IZRu7l1MS1yvNF5k5MUEEVKerjHpZWoSS8/B4eu/Mi7Y7nY4Mq2PSlGSTOPH?=
 =?us-ascii?Q?OKiuqIPSRAd9OjA3gliqAIYhyfn5GCqCNjn5Ix3KKmTEHGjidwKZhW1eyBWZ?=
 =?us-ascii?Q?ME9ToDQHL58g3g9VDaua7saWj0X5ZFoph2nlBGhVGLyhzR5IfZNVEsOxlyqF?=
 =?us-ascii?Q?vkRHYiUxDO5P5IvpvN5FHJdoUJAXkujKbiZhyixDYymflZthLO10SZYZRl/I?=
 =?us-ascii?Q?PL+a+aW8tDIokM4MzaDSyKjhe8eYzXUeKtRBd/bhBQ+E2XYiP8CNjdsiur06?=
 =?us-ascii?Q?T7McDF71kT6zYfa6Elu5h0K3ydmT1cSGPEsxVzM8+O4v1bPdXbPi5xT3Se5p?=
 =?us-ascii?Q?pcxpbLp5xixfV2yIv+HVlkMy6FyoNLqV+DQngeAFA101RLkR4zhlZYQtTdC4?=
 =?us-ascii?Q?iQ1FLXiWXpC6MY+LwTGwBxD/DZ2wbrZTZzwH8IRRyFJt/ikvmfJ0bLRlQHC3?=
 =?us-ascii?Q?geiloOITT5SjgQGOcemmL9zY1m3PxwIhJmKXEwKugsY4I906gEdKieFCGurl?=
 =?us-ascii?Q?pHz20Vykc1MxdQPDyKZhlM+CaOC1dG/9GucJLc/3ATkh/hk7GYFAyBv33GSv?=
 =?us-ascii?Q?4SkkV+ovfi8sInA9cZIzULZ34/wvNZbrLStJnQe2kuHFkf/gDr4AjrSg9K3s?=
 =?us-ascii?Q?NuYJ/OAi/mgTNMnxvmbNUk3F8F+PXm09SXOI5FD7+EbN7b9uM563FInqHoSU?=
 =?us-ascii?Q?PfxRVUGQpueKQC9NbeISQHsguRVv5isTk98St6lv6tQBUT5QH4O5K9Qur28w?=
 =?us-ascii?Q?amgRfHUbg2pIJC45tppv6bgZGgeHAHcg5Ev7EspXBmyq1KVOV+FB/xy665BO?=
 =?us-ascii?Q?aR4teZTR1Urb37t8hySxbrDKTcWK3sgkJrpw1u9MtMmOhKInsXQpdz9Lyhdr?=
 =?us-ascii?Q?WDaeDI29Zl4EGEjztgg18gpc1Ck4HUNstxwutl6VH0eFc22p7XJ1n2tTRbuQ?=
 =?us-ascii?Q?BE2CIl05uW/aOs25Qlmb4zfhDur0QcdgX9V9VCNykGHzVkvIAe6W+9FCL6+a?=
 =?us-ascii?Q?Umg7VUNwYmcFAZZ5MGpMqPUDUXz6V9+p6iC0v02oC0wFTOb2YcOmwt2mTM98?=
 =?us-ascii?Q?5MhFBbiQvkrwI+t8igIKPDFSeg9rOzPP3FH3U5FyvioDpSCn/AXyeO1i2Mh6?=
 =?us-ascii?Q?yAzUjbGRaf5Ox9OZtwCP5soSyNCXgz1XYsEfnfLYT7keg/5+i1RsFOqQ2fM2?=
 =?us-ascii?Q?pkbPeNsVFcg9eTZyw0mUTlbzoP7tFgE2AS7RAseTjZttRMGaxeXHcXYU2sCI?=
 =?us-ascii?Q?DXe0h7QnEupd4TgMULE05PaZwmEn7RIiIBhghRBCrhZTTy+qsqmveXtGivA4?=
 =?us-ascii?Q?O9g9hg=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <298630517F60E541A351A014F6B446D0@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: f4af85be-3ce0-403b-8785-08da73dafd6c
X-MS-Exchange-CrossTenant-originalarrivaltime: 01 Aug 2022 16:29:21.7534
 (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: ShA+Fa3vw7U2HFD8wpIcluIYTcbgtOoYKgaEtczxJ7qV8oz0+jlVMt2DvtoHN6hO6VqxPiH7h5otgNZPOKQZ7g==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BLAPR10MB5009
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.883,Hydra:6.0.517,FMLib:17.11.122.1
 definitions=2022-08-01_07,2022-08-01_01,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 mlxscore=0 suspectscore=0
 adultscore=0 bulkscore=0 malwarescore=0 mlxlogscore=999 phishscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2206140000
 definitions=main-2208010084
X-Proofpoint-ORIG-GUID: 0Nbnt7MINkRBnmRuuIJ2PmLJI8cysbRx
X-Proofpoint-GUID: 0Nbnt7MINkRBnmRuuIJ2PmLJI8cysbRx
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 Jul 31, 2022, at 4:19 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 the
> 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>

I'm interested in Olga's comments as well, so I'm going to
wait a bit before applying this one.

Also, did you figure out where this crash started to occur?
I'd like to have a precise sense of whether this should be
backported.


> ---
> fs/nfsd/nfs4state.c | 3 +++
> 1 file changed, 3 insertions(+)
>=20
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 9409a0dc1b76..749f51dff5c7 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -6608,6 +6608,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 +6617,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