nfsd fixes for 2.6.29

nfsd fixes for 2.6.29

[J. Bruce Fields]

The following changes are available in the git repository at:

  git://linux-nfs.org/~bfields/linux.git for-2.6.29

Mainly three regression fixes, but I also snuck in two minor
documentation updates.  (I can stop that if it's annoying.)

--b.

J. Bruce Fields (3):
      nfs: note that CONFIG_SUNRPC_XPRT_RDMA turns on server side support too
      nfsd: fix null dereference on error path
      nfsd: fix cred leak on every rpc

James Lentini (1):
      update port number in NFS/RDMA documentation

Jeff Layton (1):
      nfsd: only set file_lock.fl_lmops in nfsd4_lockt if a stateowner is found

 Documentation/filesystems/nfs-rdma.txt |    4 ++--
 fs/nfsd/auth.c                         |    3 +++
 fs/nfsd/nfs4state.c                    |    1 -
 net/sunrpc/Kconfig                     |    5 ++---
 4 files changed, 7 insertions(+), 6 deletions(-)

Re: nfsd fixes for 2.6.29

[J. Bruce Fields]

On Tue, Jan 27, 2009 at 06:20:58PM -0500, bfields wrote:
> The following changes are available in the git repository at:
> 
>   git://linux-nfs.org/~bfields/linux.git for-2.6.29
> 
> Mainly three regression fixes, but I also snuck in two minor
> documentation updates.  (I can stop that if it's annoying.)
> 
> --b.
> 
> J. Bruce Fields (3):
>       nfs: note that CONFIG_SUNRPC_XPRT_RDMA turns on server side support too
>       nfsd: fix null dereference on error path
>       nfsd: fix cred leak on every rpc
> 
> James Lentini (1):
>       update port number in NFS/RDMA documentation
> 
> Jeff Layton (1):
>       nfsd: only set file_lock.fl_lmops in nfsd4_lockt if a stateowner is found

Jeff's patch (together with the previous patch it fixes) are also
appropriate for stable; I'll follow up with the patches in question.

--b.

[PATCH] nfsd: Ensure nfsv4 calls the underlying filesystem on LOCKT

[J. Bruce Fields]

From: J. Bruce Fields <bfields@citi.umich.edu>

Since nfsv4 allows LOCKT without an open, but the ->lock() method is a
file method, we fake up a struct file in the nfsv4 code with just the
fields we need initialized.  But we forgot to initialize the file
operations, with the result that LOCKT never results in a call to the
filesystem's ->lock() method (if it exists).

We could just add that one more initialization.  But this hack of faking
up a struct file with only some fields initialized seems the kind of
thing that might cause more problems in the future.  We should either do
an open and get a real struct file, or make lock-testing an inode (not a
file) method.

This patch does the former.

Reported-by: Marc Eshel <eshel@almaden.ibm.com>
Tested-by: Marc Eshel <eshel@almaden.ibm.com>
Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
---
 fs/nfsd/nfs4state.c |   30 ++++++++++++++++++++----------
 1 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 06b89df..e62d0e3 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2782,6 +2782,25 @@ out:
 }
 
 /*
+ * The NFSv4 spec allows a client to do a LOCKT without holding an OPEN,
+ * so we do a temporary open here just to get an open file to pass to
+ * vfs_test_lock.  (Arguably perhaps test_lock should be done with an
+ * inode operation.)
+ */
+static int nfsd_test_lock(struct svc_rqst *rqstp, struct svc_fh *fhp, struct file_lock *lock)
+{
+	struct file *file;
+	int err;
+
+	err = nfsd_open(rqstp, fhp, S_IFREG, NFSD_MAY_READ, &file);
+	if (err)
+		return err;
+	err = vfs_test_lock(file, lock);
+	nfsd_close(file);
+	return err;
+}
+
+/*
  * LOCKT operation
  */
 __be32
@@ -2789,7 +2808,6 @@ nfsd4_lockt(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	    struct nfsd4_lockt *lockt)
 {
 	struct inode *inode;
-	struct file file;
 	struct file_lock file_lock;
 	int error;
 	__be32 status;
@@ -2847,16 +2865,8 @@ nfsd4_lockt(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 
 	nfs4_transform_lock_offset(&file_lock);
 
-	/* vfs_test_lock uses the struct file _only_ to resolve the inode.
-	 * since LOCKT doesn't require an OPEN, and therefore a struct
-	 * file may not exist, pass vfs_test_lock a struct file with
-	 * only the dentry:inode set.
-	 */
-	memset(&file, 0, sizeof (struct file));
-	file.f_path.dentry = cstate->current_fh.fh_dentry;
-
 	status = nfs_ok;
-	error = vfs_test_lock(&file, &file_lock);
+	error = nfsd_test_lock(rqstp, &cstate->current_fh, &file_lock);
 	if (error) {
 		status = nfserrno(error);
 		goto out;
-- 
1.5.6.3

[PATCH] nfsd: only set file_lock.fl_lmops in nfsd4_lockt if a stateowner is found

[J. Bruce Fields]

From: Jeff Layton <jlayton@redhat.com>

nfsd4_lockt does a search for a lockstateowner when building the lock
struct to test. If one is found, it'll set fl_owner to it. Regardless of
whether that happens, it'll also set fl_lmops. Given that this lock is
basically a "lightweight" lock that's just used for checking conflicts,
setting fl_lmops is probably not appropriate for it.

This behavior exposed a bug in DLM's GETLK implementation where it
wasn't clearing out the fields in the file_lock before filling in
conflicting lock info. While we were able to fix this in DLM, it
still seems pointless and dangerous to set the fl_lmops this way
when we may have a NULL lockstateowner.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: J. Bruce Fields <bfields@pig.fieldses.org>
---
 fs/nfsd/nfs4state.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 88db7d3..b6f60f4 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2871,7 +2871,6 @@ nfsd4_lockt(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		file_lock.fl_owner = (fl_owner_t)lockt->lt_stateowner;
 	file_lock.fl_pid = current->tgid;
 	file_lock.fl_flags = FL_POSIX;
-	file_lock.fl_lmops = &nfsd_posix_mng_ops;
 
 	file_lock.fl_start = lockt->lt_offset;
 	file_lock.fl_end = last_byte_offset(lockt->lt_offset, lockt->lt_length);
-- 
1.5.6.3