[PATCH] nfsd: fix race between client_info_show() and free_client()

[Ameer Hamza]

client_info_show() renders /proc/fs/nfsd/clients/<id>/info and walks
clp->cl_sessions under clp->cl_lock to print each session's slot
counts.  free_client() tears down the same list without taking
cl_lock, and is the only unlocked mutator of cl_sessions.  A reader
can observe a client mid-teardown because get_nfsdfs_clp() pins the
nfs4_client but not its sessions: free_client() frees every session
before calling nfsd_client_rmdir(), so an in-flight seq_file reader
can follow a list_del()'d node whose ->next now holds LIST_POISON1
and take a general protection fault:

  Oops: general protection fault, probably for non-canonical address
        0xdead00000000014c
  CPU: 1 UID: 0 PID: 132488 Comm: cat
  RIP: 0010:client_info_show+0x2bf/0x3d0
  RAX: dead000000000100
  Call Trace:
   seq_read_iter+0x12a/0x4b0
   seq_read+0xf1/0x130
   vfs_read+0xbf/0x350
   ksys_read+0x6f/0xf0
   do_syscall_64+0x8b/0xcb0
   entry_SYSCALL_64_after_hwframe+0x76/0x7e
  Kernel panic - not syncing: Fatal exception

Detach cl_sessions onto a local reaplist under cl_lock, then free the
sessions after dropping the lock.  Removing entries from cl_sessions
under cl_lock matches unhash_session(), and the detach-then-reap shape
matches how __destroy_client() reaps cl_delegations.  The sessions
cannot be freed while cl_lock is held, since free_session() calls
nfsd4_del_conns(), which re-acquires it.

Reported-by: Nicholas Wolff <nicholas.wolff@xxxxxxxxxxx>
Fixes: 601c8cb349c2 ("nfsd: add session slot count to /proc/fs/nfsd/clients/*/info")
Signed-off-by: Ameer Hamza <ameer.hamza@xxxxxxxxxxx>
---
Reproducer, on a throwaway NFSv4.1 server (panics within a couple of
minutes without this patch):

  sysctl -w kernel.panic_on_oops=1
  mkdir -p /export /mnt/loopback
  echo '/export 127.0.0.1(rw,no_root_squash,fsid=0)' >/etc/exports
  systemctl start nfs-server; exportfs -ra
  mount -t nfs -o vers=4.1 127.0.0.1:/ /mnt/loopback

  for i in $(seq $(nproc)); do
      ( while :; do cat /proc/fs/nfsd/clients/*/info >/dev/null 2>&1; done ) &
  done

  shopt -s nullglob
  while :; do
      ls /mnt/loopback >/dev/null 2>&1
      for c in /proc/fs/nfsd/clients/*/; do printf 'expire\n' >"${c}ctl"; done
  done

The "expire" write is what drives free_client(); a reader-only loop
does not reproduce this. panic_on_oops is needed because the fault
happens while the reader holds cl_lock.

 fs/nfsd/nfs4state.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index d7731bb4959c..d93672e6fa26 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2791,10 +2791,16 @@ void nfsd4_put_client(struct nfs4_client *clp)
 static void
 free_client(struct nfs4_client *clp)
 {
-	while (!list_empty(&clp->cl_sessions)) {
+	LIST_HEAD(reaplist);
+
+	/* client_info_show() walks cl_sessions under cl_lock */
+	spin_lock(&clp->cl_lock);
+	list_splice_init(&clp->cl_sessions, &reaplist);
+	spin_unlock(&clp->cl_lock);
+	while (!list_empty(&reaplist)) {
 		struct nfsd4_session *ses;
-		ses = list_entry(clp->cl_sessions.next, struct nfsd4_session,
-				se_perclnt);
+		ses = list_entry(reaplist.next, struct nfsd4_session,
+				 se_perclnt);
 		list_del(&ses->se_perclnt);
 		WARN_ON_ONCE(atomic_read(&ses->se_ref));
 		free_session(ses);

base-commit: 8d4fbec07074c98f4b16c7e38964227fdf4d5cd3
-- 
2.53.0