[PATCH] SUNRPC: Fix use-after-free in svc_age_temp_xprts_now()

[Chuck Lever <cel@kernel.org>]

svc_age_temp_xprts_now() moves each matching temporary transport
onto a local list under sv_lock, then drops the lock and walks the
local list without holding a reference on any of them. In the
meantime svc_delete_xprt() can take sv_lock, unlink the same
xpt_list node, and drop the transport's last reference. An idle
temporary transport holds only that one reference, so a peer FIN
that arrives between the two loops frees the svc_sock out from under
the second loop. The notifier then sets XPT_CLOSE in freed memory,
and svc_xprt_enqueue() reads xpt_flags and xpt_server from the freed
object and queues that object on a pool for an nfsd thread. KASAN
reports a slab-use-after-free in svc_xprt_enqueue() under
nfsd_inetaddr_event(), with the object freed by svc_sock_free() on
an nfsd thread.

The two-phase walk dates from when the second loop called into the
socket layer and could sleep. Commit 546125d16142 ("sunrpc: don't
call sleeping functions from the notifier block callbacks") moved
that work to the nfsd threads, so nothing left in the second loop
needs sv_lock dropped. Mark and enqueue each matching transport
under sv_lock in a single pass, as svc_age_temp_xprts() already
does, so that svc_delete_xprt() cannot unlink and free a transport
the notifier is still touching.

Fixes: c3d4879e01be ("sunrpc: Add a function to close temporary transports immediately")
Reported-by: Mohammed Alquraini <mohammed.saad.alquraini@gmail.com>
Signed-off-by: Chuck Lever <cel@kernel.org>
---
 net/sunrpc/svc_xprt.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

Mohammed, thanks for the report. Please test and report your results.

diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
index 5db42e53be62..076c624fe675 100644
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -1075,29 +1075,21 @@ void svc_age_temp_xprts_now(struct svc_serv *serv, struct sockaddr *server_addr)
 {
 	struct svc_xprt *xprt;
 	struct list_head *le, *next;
-	LIST_HEAD(to_be_closed);
 
 	spin_lock_bh(&serv->sv_lock);
 	list_for_each_safe(le, next, &serv->sv_tempsocks) {
 		xprt = list_entry(le, struct svc_xprt, xpt_list);
-		if (rpc_cmp_addr(server_addr, (struct sockaddr *)
-				&xprt->xpt_local)) {
-			dprintk("svc_age_temp_xprts_now: found %p\n", xprt);
-			list_move(le, &to_be_closed);
-		}
-	}
-	spin_unlock_bh(&serv->sv_lock);
-
-	while (!list_empty(&to_be_closed)) {
-		le = to_be_closed.next;
+		if (!rpc_cmp_addr(server_addr, (struct sockaddr *)
+				&xprt->xpt_local))
+			continue;
 		list_del_init(le);
-		xprt = list_entry(le, struct svc_xprt, xpt_list);
 		set_bit(XPT_CLOSE, &xprt->xpt_flags);
 		set_bit(XPT_KILL_TEMP, &xprt->xpt_flags);
 		dprintk("svc_age_temp_xprts_now: queuing xprt %p for closing\n",
 				xprt);
 		svc_xprt_enqueue(xprt);
 	}
+	spin_unlock_bh(&serv->sv_lock);
 }
 EXPORT_SYMBOL_GPL(svc_age_temp_xprts_now);
 
-- 
2.54.0