From: Jeff Layton <jlayton@redhat.com>
Date: Thu, 5 Nov 2009 08:32:09 -0500
Subject: [nfs] v4: fix setting lock on open file with no state
Message-id: 1257427929-19106-1-git-send-email-jlayton@redhat.com
O-Subject: [RHEL5.5 PATCH] BZ#533115: NFSv4: fix oops when setting lock on open file with no state
Bugzilla: 533115
RH-Acked-by: Peter Staubach <staubach@redhat.com>
CVE: CVE-2009-3726

From: Trond Myklebust <Trond.Myklebust@netapp.com>

It's possible for an unprivileged user to cause an oops on a host where
they have r/w access to a NFSv4 mount. All they have to do is have a
program open /proc/self/exe and then try to get a POSIX lock on that
file descriptor.

For some reason that I'm still investigating, opening /proc/self/exe
doesn't cause the link target to be revalidated in the dcache. This
makes NFSv4 allow the open to proceed without attaching any NFSv4 state
to its open context leaving that pointer as NULL. The lock code assumes
that that pointer will be valid however and dereferences it, causing an
oops. This patch simply has it check whether the pointer is valid before
dereferencing it. If it's not, then the SETLK operation returns -ENOLCK.

In the reproducer we have the lock ought to succeed, so this isn't yet a
"real" fix for the root cause. I'm pusuing that now, but it'll need to
go upstream first. In the meantime, this patch is backported from an
upstream patch that prevents the oops there.

Confirmed by testing with the reproducer I have for this.

Original patch description follows:

------------------[snip]----------------

(upstream commit d953126a28f97ec965d23c69fd5795854c048f30)

We just had a case in which a buggy server occasionally returns the
wrong attributes during an OPEN call. While the client does catch this
sort of condition in nfs4_open_done(), and causes the nfs4_atomic_open()
to return -EISDIR, the logic in nfs_atomic_lookup() is broken, since it
causes a fallback to an ordinary lookup instead of just returning the
error.

When the buggy server then returns a regular file for the fallback
lookup, the VFS allows the open, and bad things start to happen, since
the open file doesn't have any associated NFSv4 state.

The fix is firstly to return the EISDIR/ENOTDIR errors immediately, and
secondly to ensure that we are always careful when dereferencing the
nfs_open_context state pointer.

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 6ce4ab9..5ff4dbc 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -995,12 +995,12 @@ static struct dentry *nfs_atomic_lookup(struct inode *dir, struct dentry *dentry
 				res = NULL;
 				goto out;
 			/* This turned out not to be a regular file */
-			case -EISDIR:
 			case -ENOTDIR:
 				goto no_open;
 			case -ELOOP:
 				if (!(nd->intent.open.flags & O_NOFOLLOW))
 					goto no_open;
+			/* case -EISDIR: */
 			/* case -EINVAL: */
 			default:
 				goto out;
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 7f2e0b9..572af58 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -3627,15 +3627,23 @@ nfs4_proc_lock(struct file *filp, int cmd, struct file_lock *request)
 	if (request->fl_start < 0 || request->fl_end < 0)
 		return -EINVAL;
 
-	if (IS_GETLK(cmd))
-		return nfs4_proc_getlk(state, F_GETLK, request);
+	if (IS_GETLK(cmd)) {
+		if (state != NULL)
+			return nfs4_proc_getlk(state, F_GETLK, request);
+		return 0;
+	}
 
 	if (!(IS_SETLK(cmd) || IS_SETLKW(cmd)))
 		return -EINVAL;
 
-	if (request->fl_type == F_UNLCK)
-		return nfs4_proc_unlck(state, cmd, request);
+	if (request->fl_type == F_UNLCK) {
+		if (state != NULL)
+			return nfs4_proc_unlck(state, cmd, request);
+		return 0;
+	}
 
+	if (state == NULL)
+		return -ENOLCK;
 	do {
 		status = nfs4_proc_setlk(state, cmd, request);
 		if ((status != -EAGAIN) || IS_SETLK(cmd))