Patch "SUNRPC: fix refcounting problems with auth_gss messages." has been added to the 4.9-stable tree

This is a note to let you know that I've just added the patch titled

SUNRPC: fix refcounting problems with auth_gss messages.

to the 4.9-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
sunrpc-fix-refcounting-problems-with-auth_gss-messages.patch
and it can be found in the queue-4.9 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

From: NeilBrown <[email protected]>
Date: Mon, 5 Dec 2016 15:10:11 +1100
Subject: SUNRPC: fix refcounting problems with auth_gss messages.

From: NeilBrown <[email protected]>

commit 1cded9d2974fe4fe339fc0ccd6638b80d465ab2c upstream.

There are two problems with refcounting of auth_gss messages.

First, the reference on the pipe->pipe list (taken by a call to rpc_queue_upcall()) is not counted. It seems to be assumed that a message in pipe->pipe will always also be in pipe->in_downcall, where it is correctly reference counted.

However there is no guaranty of this. I have a report of a NULL dereferences in rpc_pipe_read() which suggests a msg that has been freed is still on the pipe->pipe list.

One way I imagine this might happen is:
- message is queued for uid=U and auth->service=S1
- rpc.gssd reads this message and starts processing.
This removes the message from pipe->pipe
- message is queued for uid=U and auth->service=S2
- rpc.gssd replies to the first message. gss_pipe_downcall() calls __gss_find_upcall(pipe, U, NULL) and it finds the *second* message, as new messages are placed at the head of ->in_downcall, and the service type is not checked.
- This second message is removed from ->in_downcall and freed by gss_release_msg() (even though it is still on pipe->pipe)
- rpc.gssd tries to read another message, and dereferences a pointer to this message that has just been freed.

I fix this by incrementing the reference count before calling rpc_queue_upcall(), and decrementing it if that fails, or normally in gss_pipe_destroy_msg().

It seems strange that the reply doesn't target the message more precisely, but I don't know all the details. In any case, I think the reference counting irregularity became a measureable bug when the extra arg was added to __gss_find_upcall(), hence the Fixes: line below.

The second problem is that if rpc_queue_upcall() fails, the new message is not freed. gss_alloc_msg() set the ->count to 1, gss_add_msg() increments this to 2, gss_unhash_msg() decrements to 1, then the pointer is discarded so the memory never gets freed.

---
 net/sunrpc/auth_gss/auth_gss.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

@@ -541,9 +541,13 @@ gss_setup_upcall(struct gss_auth *gss_au
 		return gss_new;
 	gss_msg = gss_add_msg(gss_new);
 	if (gss_msg == gss_new) {
-		int res = rpc_queue_upcall(gss_new->pipe, &gss_new->msg);
+		int res;
+		atomic_inc(&gss_msg->count);
+		res = rpc_queue_upcall(gss_new->pipe, &gss_new->msg);
 		if (res) {
 			gss_unhash_msg(gss_new);
+			atomic_dec(&gss_msg->count);
+			gss_release_msg(gss_new);
 			gss_msg = ERR_PTR(res);
 		}
 	} else

@@ -836,6 +840,7 @@ gss_pipe_destroy_msg(struct rpc_pipe_msg
 			warn_gssd();
 		gss_release_msg(gss_msg);
 	}
+	gss_release_msg(gss_msg);
 }
 
 static void gss_pipe_dentry_destroy(struct dentry *dir,

Patches currently in stable-queue which might be from [email protected] are

queue-4.9/md-raid5-limit-request-size-according-to-implementation-limits.patch queue-4.9/sunrpc-fix-refcounting-problems-with-auth_gss-messages.patch