[PATCH v2 1/6] NFSD: Prevent lock owner use-after-free during client teardown
Chuck Lever <[email protected]>
| Newsgroups | gmane.linux.nfs |
|---|---|
| Message-ID | <[email protected]> |
After __destroy_client() releases a client's open owners, a lock owner whose only remaining reference is a blocked lock (nbl) is left on cl_ownerstr_hashtbl. client_has_state() does not account for a bare lock owner, so DESTROY_CLIENTID can reach __destroy_client() with these lock owners still present. __destroy_client() then walks cl_ownerstr_hashtbl and calls remove_blocked_locks() on each lock owner without holding a reference. Freeing a blocked lock drops the lock owner reference held through the file_lock's flc_owner, so the per-net laundromat, which reaps timed-out blocked locks from nn->blocked_locks_lru independently of client state, can free the same lock owner concurrently. The two paths serialize on blocked_locks_lock for the list splice only, not for the lock owner's lifetime. The laundromat can therefore free the lock owner while __destroy_client() is about to dereference it, and the freed, zeroed slab object produces a NULL dereference in remove_blocked_locks(). nfsd4_release_lockowner() holds a reference across the same call; __destroy_client() does not. Hold cl_lock across the walk, and take a reference and unhash each lock owner before dropping the lock, so the laundromat cannot reap a blocked lock and free the lock owner underneath this loop. cl_lock is released before remove_blocked_locks() and nfs4_put_stateowner(), which take blocked_locks_lock and cl_lock respectively. Reported-by: Wolfgang Walter <[email protected]> Closes: https://lore.kernel.org/linux-nfs/[email protected]/ Fixes: 68ef3bc31664 ("nfsd: remove blocked locks on client teardown") Signed-off-by: Chuck Lever <[email protected]> --- fs/nfsd/nfs4state.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index a4398dc861a5..e000ed3e96e9 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -2758,14 +2758,24 @@ __destroy_client(struct nfs4_client *clp) release_openowner(oo); } for (i = 0; i < OWNER_HASH_SIZE; i++) { - struct nfs4_stateowner *so, *tmp; + struct nfs4_stateowner *so; - list_for_each_entry_safe(so, tmp, &clp->cl_ownerstr_hashtbl[i], - so_strhash) { + spin_lock(&clp->cl_lock); + while (!list_empty(&clp->cl_ownerstr_hashtbl[i])) { + so = list_first_entry(&clp->cl_ownerstr_hashtbl[i], + struct nfs4_stateowner, so_strhash); /* Should be no openowners at this point */ WARN_ON_ONCE(so->so_is_open_owner); + nfs4_get_stateowner(so); + unhash_lockowner_locked(lockowner(so)); + spin_unlock(&clp->cl_lock); + remove_blocked_locks(lockowner(so)); + nfs4_put_stateowner(so); + + spin_lock(&clp->cl_lock); } + spin_unlock(&clp->cl_lock); } nfsd4_return_all_client_layouts(clp); nfsd4_shutdown_copy(clp); -- 2.54.0