Skip to contentSkip to conversation

NFSD: Prevent lock owner use-after-free during client teardown

RFC PATCH Superseded Chuck Lever 1 participant 1 message Jul 3

Subsystems (2)

v1 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Chuck Lever

author maintainer
#0 lore raw

RFC PATCH 1/1 NFSD: Prevent lock owner use-after-free during client teardown

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.

Fixes 68ef3bc31664 ("nfsd: remove blocked locks on client teardown")
---
 fs/nfsd/nfs4state.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)
fs/nfsd/nfs4state.c +13 -3
index a4398dc861a5..e000ed3e96e9 100644
@@ -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);
Signature · 1 line
2.54.0