SUNRPC: Fix a use after free bug with the NFSv4.1 backchannel

The ->release_request() callback was designed to allow the transport layer to do housekeeping after the RPC call is done. It cannot be used to free the request itself, and doing so leads to a use-after-free bug in xprt_release(). Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>

SUNRPC: Fix a use after free bug with the NFSv4.1 backchannel
The ->release_request() callback was designed to allow the transport layer to do housekeeping after the RPC call is done. It cannot be used to free the request itself, and doing so leads to a use-after-free bug in xprt_release(). Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
Trond Myklebust
1 parent cdead7cf12
Showing 4 changed files with 13 additions and 32 deletions Side-by-side Diff
include/linux/sunrpc/bc_xprt.h
net/sunrpc/bc_svc.c
net/sunrpc/xprt.c
net/sunrpc/xprtsock.c
@@ -36,7 +36,6 @@
 void xprt_free_bc_request(struct rpc_rqst *req);
 int xprt_setup_backchannel(struct rpc_xprt *, unsigned int min_reqs);
 void xprt_destroy_backchannel(struct rpc_xprt *, int max_reqs);
-void bc_release_request(struct rpc_task *);
 int bc_send(struct rpc_rqst *req);
  
 /*
@@ -58,6 +57,10 @@
 static inline int svc_is_backchannel(const struct svc_rqst *rqstp)
 {
 	return 0;
+}
+
+static inline void xprt_free_bc_request(struct rpc_rqst *req)
+{
 }
 #endif /* CONFIG_NFS_V4_1 */
 #endif /* _LINUX_SUNRPC_BC_XPRT_H */
@@ -37,21 +37,6 @@
  
 #define RPCDBG_FACILITY	RPCDBG_SVCDSP
  
-void bc_release_request(struct rpc_task *task)
-{
-	struct rpc_rqst *req = task->tk_rqstp;
-
-	dprintk("RPC:       bc_release_request: task= %p\n", task);
-
-	/*
-	 * Release this request only if it's a backchannel
-	 * preallocated request
-	 */
-	if (!bc_prealloc(req))
-		return;
-	xprt_free_bc_request(req);
-}
-
 /* Empty callback ops */
 static const struct rpc_call_ops nfs41_callback_ops = {
 };
@@ -46,6 +46,7 @@
  
 #include <linux/sunrpc/clnt.h>
 #include <linux/sunrpc/metrics.h>
+#include <linux/sunrpc/bc_xprt.h>
  
 #include "sunrpc.h"
  
  
  
@@ -1032,21 +1033,16 @@
 	if (req->rq_release_snd_buf)
 		req->rq_release_snd_buf(req);
  
-	/*
-	 * Early exit if this is a backchannel preallocated request.
-	 * There is no need to have it added to the RPC slot list.
-	 */
-	if (is_bc_request)
-		return;
-
-	memset(req, 0, sizeof(*req));	/* mark unused */
-
 	dprintk("RPC: %5u release request %p\n", task->tk_pid, req);
+	if (likely(!is_bc_request)) {
+		memset(req, 0, sizeof(*req));	/* mark unused */
  
-	spin_lock(&xprt->reserve_lock);
-	list_add(&req->rq_list, &xprt->free);
-	rpc_wake_up_next(&xprt->backlog);
-	spin_unlock(&xprt->reserve_lock);
+		spin_lock(&xprt->reserve_lock);
+		list_add(&req->rq_list, &xprt->free);
+		rpc_wake_up_next(&xprt->backlog);
+		spin_unlock(&xprt->reserve_lock);
+	} else
+		xprt_free_bc_request(req);
 }
  
 /**
@@ -2251,9 +2251,6 @@
 	.buf_free		= rpc_free,
 	.send_request		= xs_tcp_send_request,
 	.set_retrans_timeout	= xprt_set_retrans_timeout_def,
-#if defined(CONFIG_NFS_V4_1)
-	.release_request	= bc_release_request,
-#endif /* CONFIG_NFS_V4_1 */
 	.close			= xs_tcp_close,
 	.destroy		= xs_destroy,
 	.print_stats		= xs_tcp_print_stats,
...	...	@@ -36,7 +36,6 @@
36	36	void xprt_free_bc_request(struct rpc_rqst *req);
37	37	int xprt_setup_backchannel(struct rpc_xprt *, unsigned int min_reqs);
38	38	void xprt_destroy_backchannel(struct rpc_xprt *, int max_reqs);
39		-void bc_release_request(struct rpc_task *);
40	39	int bc_send(struct rpc_rqst *req);
41	40
42	41	/*
...	...	@@ -58,6 +57,10 @@
58	57	static inline int svc_is_backchannel(const struct svc_rqst *rqstp)
59	58	{
60	59	return 0;
	60	+}
	61	+
	62	+static inline void xprt_free_bc_request(struct rpc_rqst *req)
	63	+{
61	64	}
62	65	#endif /* CONFIG_NFS_V4_1 */
63	66	#endif /* _LINUX_SUNRPC_BC_XPRT_H */
...	...	@@ -37,21 +37,6 @@
37	37
38	38	#define RPCDBG_FACILITY RPCDBG_SVCDSP
39	39
40		-void bc_release_request(struct rpc_task *task)
41		-{
42		- struct rpc_rqst *req = task->tk_rqstp;
43		-
44		- dprintk("RPC: bc_release_request: task= %p\n", task);
45		-
46		- /*
47		- * Release this request only if it's a backchannel
48		- * preallocated request
49		- */
50		- if (!bc_prealloc(req))
51		- return;
52		- xprt_free_bc_request(req);
53		-}
54		-
55	40	/* Empty callback ops */
56	41	static const struct rpc_call_ops nfs41_callback_ops = {
57	42	};
...	...	@@ -46,6 +46,7 @@
46	46
47	47	#include <linux/sunrpc/clnt.h>
48	48	#include <linux/sunrpc/metrics.h>
	49	+#include <linux/sunrpc/bc_xprt.h>
49	50
50	51	#include "sunrpc.h"
51	52
52	53
53	54
...	...	@@ -1032,21 +1033,16 @@
1032	1033	if (req->rq_release_snd_buf)
1033	1034	req->rq_release_snd_buf(req);
1034	1035
1035		- /*
1036		- * Early exit if this is a backchannel preallocated request.
1037		- * There is no need to have it added to the RPC slot list.
1038		- */
1039		- if (is_bc_request)
1040		- return;
1041		-
1042		- memset(req, 0, sizeof(req)); / mark unused */
1043		-
1044	1036	dprintk("RPC: %5u release request %p\n", task->tk_pid, req);
	1037	+ if (likely(!is_bc_request)) {
	1038	+ memset(req, 0, sizeof(req)); / mark unused */
1045	1039
1046		- spin_lock(&xprt->reserve_lock);
1047		- list_add(&req->rq_list, &xprt->free);
1048		- rpc_wake_up_next(&xprt->backlog);
1049		- spin_unlock(&xprt->reserve_lock);
	1040	+ spin_lock(&xprt->reserve_lock);
	1041	+ list_add(&req->rq_list, &xprt->free);
	1042	+ rpc_wake_up_next(&xprt->backlog);
	1043	+ spin_unlock(&xprt->reserve_lock);
	1044	+ } else
	1045	+ xprt_free_bc_request(req);
1050	1046	}
1051	1047
1052	1048	/**
...	...	@@ -2251,9 +2251,6 @@
2251	2251	.buf_free = rpc_free,
2252	2252	.send_request = xs_tcp_send_request,
2253	2253	.set_retrans_timeout = xprt_set_retrans_timeout_def,
2254		-#if defined(CONFIG_NFS_V4_1)
2255		- .release_request = bc_release_request,
2256		-#endif /* CONFIG_NFS_V4_1 */
2257	2254	.close = xs_tcp_close,
2258	2255	.destroy = xs_destroy,
2259	2256	.print_stats = xs_tcp_print_stats,