Re: [PATCH] NFSD: fix use-after-free on source server when doing inter-server copy

[Chuck Lever III <chuck.lever@oracle.com>]

> On Jul 31, 2022, at 4:19 PM, Dai Ngo <dai.ngo@oracle.com> wrote:
> 
> 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.
> 
> 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.
> 
> 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.
> 
> 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.
> 
> 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(+)
> 
> 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 = s->st_stid.sc_client;
> 	bool unhashed;
> 	LIST_HEAD(reaplist);
> +	struct nfs4_ol_stateid *stp;
> 
> 	spin_lock(&clp->cl_lock);
> 	unhashed = 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);
> -- 
> 2.9.5
> 

--
Chuck Lever