[linux] 01/01: sunrpc: fix refcounting problems with auth_gss messages

Tue Feb 28 11:20:02 UTC 2017

This is an automated email from the git hooks/post-receive script.

carnil pushed a commit to branch jessie
in repository linux.

commit da6af8dc1b6eddb4e3ec6ef2fd482491cc70bcef
Author: Salvatore Bonaccorso <carnil at debian.org>
Date:   Tue Feb 28 10:59:04 2017 +0100

    sunrpc: fix refcounting problems with auth_gss messages
    
    Thanks: Raphael Geissert <geissert at debian.org>
    Closes: #852708
---
 debian/changelog                                   |  5 ++
 ...refcounting-problems-with-auth_gss-messag.patch | 85 ++++++++++++++++++++++
 debian/patches/series                              |  1 +
 3 files changed, 91 insertions(+)

diff --git a/debian/changelog b/debian/changelog
index ff22c3b..0d5d8dc 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,7 +1,12 @@
 linux (3.16.39-2) UNRELEASED; urgency=medium
 
+  [ Ben Hutchings ]
   * locking/mutex: Don't assume TASK_RUNNING (Closes: #841171)
 
+  [ Salvatore Bonaccorso ]
+  * sunrpc: fix refcounting problems with auth_gss messages.
+    Thanks to Raphael Geissert <geissert at debian.org> (Closes: #852708)
+
  -- Ben Hutchings <ben at decadent.org.uk>  Sun, 01 Jan 2017 22:44:31 +0000
 
 linux (3.16.39-1+deb8u1) jessie-security; urgency=high
diff --git a/debian/patches/bugfix/all/SUNRPC-fix-refcounting-problems-with-auth_gss-messag.patch b/debian/patches/bugfix/all/SUNRPC-fix-refcounting-problems-with-auth_gss-messag.patch
new file mode 100644
index 0000000..72df615
--- /dev/null
+++ b/debian/patches/bugfix/all/SUNRPC-fix-refcounting-problems-with-auth_gss-messag.patch
@@ -0,0 +1,85 @@
+From: NeilBrown <neilb at suse.com>
+Date: Mon, 5 Dec 2016 15:10:11 +1100
+Subject: SUNRPC: fix refcounting problems with auth_gss messages.
+Origin: https://git.kernel.org/linus/1cded9d2974fe4fe339fc0ccd6638b80d465ab2c
+Bug-Debian: https://bugs.debian.org/852708
+
+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.
+
+Fixes: 9130b8dbc6ac ("SUNRPC: allow for upcalls for same uid but different gss service")
+Cc: stable at vger.kernel.org
+Link: https://bugzilla.opensuse.org/show_bug.cgi?id=1011250
+Signed-off-by: NeilBrown <neilb at suse.com>
+Signed-off-by: Trond Myklebust <trond.myklebust at primarydata.com>
+---
+ net/sunrpc/auth_gss/auth_gss.c | 7 ++++++-
+ 1 file changed, 6 insertions(+), 1 deletion(-)
+
+diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c
+index 3dfd769dc5b5..16cea00c959b 100644
+--- a/net/sunrpc/auth_gss/auth_gss.c
++++ b/net/sunrpc/auth_gss/auth_gss.c
+@@ -541,9 +541,13 @@ gss_setup_upcall(struct gss_auth *gss_auth, struct rpc_cred *cred)
+ 		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 *msg)
+ 			warn_gssd();
+ 		gss_release_msg(gss_msg);
+ 	}
++	gss_release_msg(gss_msg);
+ }
+ 
+ static void gss_pipe_dentry_destroy(struct dentry *dir,
+-- 
+2.11.0
+
diff --git a/debian/patches/series b/debian/patches/series
index 43ccd0f..00241ec 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -247,6 +247,7 @@ bugfix/all/fuse-propagate-dentry-down-to-inode_change_ok.patch
 bugfix/all/fs-give-dentry-to-inode_change_ok-instead-of-inode.patch
 bugfix/all/-xen-blkfront-fix-accounting-of-reqs-when-migrating.patch
 bugfix/all/locking-mutex-don-t-assume-task_running.patch
+bugfix/all/SUNRPC-fix-refcounting-problems-with-auth_gss-messag.patch
 
 # memfd_create() & kdbus backport
 features/all/kdbus/mm-allow-drivers-to-prevent-new-writable-mappings.patch

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/kernel/linux.git

