2022-09-26 18:13:03

by Dai Ngo

Subject: [PATCH v3 0/1] NFSD: fix use-after-free on source server when doing inter-server copy

Hi Chuck,

The v3 version simplifies the check for sc_cp_list empty in
nfs4_free_deleg and nfs4_free_ol_stateid, as Jeff pointed out.

Thanks,

Dai Ngo (1):
NFSD: fix use-after-free on source server when doing inter-server copy

fs/nfsd/nfs4state.c | 5 +++++
1 file changed, 5 insertions(+)

2022-09-26 18:13:17

by Dai Ngo

Subject: [PATCH v3 1/1] NFSD: fix use-after-free on source server when doing inter-server copy

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 <[email protected]>
---
fs/nfsd/nfs4state.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index c5d199d7e6b4..0bc36472f8b7 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1049,6 +1049,7 @@ static struct nfs4_ol_stateid * nfs4_alloc_open_stateid(struct nfs4_client *clp)

static void nfs4_free_deleg(struct nfs4_stid *stid)
{
+ WARN_ON(!list_empty(&stid->sc_cp_list));
kmem_cache_free(deleg_slab, stid);
atomic_long_dec(&num_delegations);
}
@@ -1462,6 +1463,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(&stid->sc_cp_list));
kmem_cache_free(stateid_slab, stid);
}

@@ -6684,6 +6686,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);
@@ -6692,6 +6695,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

2022-09-26 18:46:25

by Chuck Lever III

Subject: Re: [PATCH v3 0/1] NFSD: fix use-after-free on source server when doing inter-server copy

> On Sep 26, 2022, at 1:59 PM, Dai Ngo <[email protected]> wrote:
>
> Hi Chuck,
>
> The v3 version simplifies the check for sc_cp_list empty in
> nfs4_free_deleg and nfs4_free_ol_stateid, as Jeff pointed out.
>
> Thanks,
>
> Dai Ngo (1):
> NFSD: fix use-after-free on source server when doing inter-server copy
>
> fs/nfsd/nfs4state.c | 5 +++++
> 1 file changed, 5 insertions(+)

Thanks, Dai. (Re)applied to nfsd's for-next.


--
Chuck Lever