[PATCH] sunrpc: bug on rpc_task_set_client when no client is present.

If we pass a NULL client to rpc_task_set_client and no client is
attached to the task, then the kernel will crash later. Antecipate the
crash by checking if a client is available for the task.

Signed-off-by: Thiago Rafael Becker <redacted>
---
 net/sunrpc/clnt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index f056ff931444..ccbc9a9715da 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c

@@ -1076,7 +1076,7 @@ void rpc_task_set_transport(struct rpc_task *task, struct rpc_clnt *clnt)
 static
 void rpc_task_set_client(struct rpc_task *task, struct rpc_clnt *clnt)
 {
-
+	BUG_ON(clnt == NULL && task->tk_client == NULL);
 	if (clnt != NULL) {
 		rpc_task_set_transport(task, clnt);
 		task->tk_client = clnt;

-- 
2.31.1

On Mon, 2021-10-18 at 09:38 -0300, Thiago Rafael Becker wrote:

I'm not seeing the point of this BUG_ON(). Why not just change this
code to not check for clnt == NULL, and let the thing Oops when it
tries to dereference clnt?

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com

On Wed, Oct 20, 2021 at 07:04:35PM +0000, Trond Myklebust wrote:
Hello,

I'm not seeing the point of this BUG_ON(). Why not just change this
code to not check for clnt == NULL, and let the thing Oops when it
tries to dereference clnt?

This was changed in 58f9612c6ea85, prior to that, this was not tested. I'm
not sure why this test exists, the only reason I can imagine is to keep
the previous task's rpc_client in case the new client is NULL. Decided
to go conservative on this, and BUG_ON() when no client is available.

Inside the linux source, I don't see how this may happen unless the code
has a bug, so I think it's possible to remove this test.

-- Thiago
 

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com