mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Daniel J Blueman" <daniel.blueman@gmail.com>
To: "Trond Myklebust" <trond.myklebust@fys.uio.no>,
	"Linus Torvalds" <torvalds@osdl.org>
Cc: "Linux Kernel" <linux-kernel@vger.kernel.org>,
	nfsv4@linux-nfs.org, "Jeff Garzik" <jeff@garzik.org>,
	"Neil Brown" <neilb@suse.de>,
	"J. Bruce Fields" <bfields@fieldses.org>
Subject: Fwd: [PATCH 2/2] nfsd4: fix open-create permissions
Date: Mon, 6 Nov 2006 16:48:23 +0000	[thread overview]
Message-ID: <6278d2220611060848n3585ebc5odbf39efd6a02ab2@mail.gmail.com> (raw)
In-Reply-To: <20061106162458.GC12372@fieldses.org>

Linus, Trond,

What is the chance of this patch making it into the final 2.6.19?

WIthout it, there is a serious NFSv4 open() regression; I've been
running it on client and server for ~1 week under load and it resolves
the condition w/o side-effects. See the LKML thread "Poor NFSv4 first
impressions" for further details.

Daniel

---------- Forwarded message ----------
From: J. Bruce Fields <bfields@fieldses.org>
Date: 06-Nov-2006 16:24
Subject: [PATCH 2/2] nfsd4: fix open-create permissions
To: Daniel J Blueman <daniel.blueman@gmail.com>
Cc: nfsv4@linux-nfs.org, Linux Kernel <linux-kernel@vger.kernel.org>,
Jeff Garzik <jeff@garzik.org>, Neil Brown <neilb@suse.de>


In the case where an open creates the file, we shouldn't be rechecking
permissions to open the file; the open succeeds regardless of what the
new file's mode bits say.

This patch fixes the problem, but only by introducing yet another parameter
to nfsd_create_v3.  This is ugly.  This will be fixed by later patches.

Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
---
 fs/nfsd/nfs3proc.c        |    2 +-
 fs/nfsd/nfs4proc.c        |    6 ++++--
 fs/nfsd/vfs.c             |    4 +++-
 include/linux/nfsd/nfsd.h |    2 +-
 4 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c
index 64db601..7f5bad0 100644
--- a/fs/nfsd/nfs3proc.c
+++ b/fs/nfsd/nfs3proc.c
@@ -258,7 +258,7 @@ nfsd3_proc_create(struct svc_rqst *rqstp
        /* Now create the file and set attributes */
        nfserr = nfsd_create_v3(rqstp, dirfhp, argp->name, argp->len,
                                attr, newfhp,
-                               argp->createmode, argp->verf, NULL);
+                               argp->createmode, argp->verf, NULL, NULL);

        RETURN_STATUS(nfserr);
 }
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 4a73f5b..50bc942 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -93,6 +93,7 @@ do_open_lookup(struct svc_rqst *rqstp, s
 {
        struct svc_fh resfh;
        __be32 status;
+       int created = 0;

        fh_init(&resfh, NFS4_FHSIZE);
        open->op_truncate = 0;
@@ -105,7 +106,7 @@ do_open_lookup(struct svc_rqst *rqstp, s
                status = nfsd_create_v3(rqstp, current_fh, open->op_fname.data,
                                        open->op_fname.len, &open->op_iattr,
                                        &resfh, open->op_createmode,
-                                       (u32 *)open->op_verf.data,
&open->op_truncate);
+                                       (u32 *)open->op_verf.data,
&open->op_truncate, &created);
        } else {
                status = nfsd_lookup(rqstp, current_fh,
                                     open->op_fname.data,
open->op_fname.len, &resfh);
@@ -122,7 +123,8 @@ do_open_lookup(struct svc_rqst *rqstp, s
        memcpy(open->op_stateowner->so_replay.rp_openfh,
                        &resfh.fh_handle.fh_base, resfh.fh_handle.fh_size);

-       status = do_open_permission(rqstp, current_fh, open, MAY_NOP);
+       if (!created)
+               status = do_open_permission(rqstp, current_fh, open, MAY_NOP);

 out:
        fh_put(&resfh);
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index f21e917..1a7ad8c 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1237,7 +1237,7 @@ __be32
 nfsd_create_v3(struct svc_rqst *rqstp, struct svc_fh *fhp,
                char *fname, int flen, struct iattr *iap,
                struct svc_fh *resfhp, int createmode, u32 *verifier,
-               int *truncp)
+               int *truncp, int *created)
 {
        struct dentry   *dentry, *dchild = NULL;
        struct inode    *dirp;
@@ -1331,6 +1331,8 @@ nfsd_create_v3(struct svc_rqst *rqstp, s
        host_err = vfs_create(dirp, dchild, iap->ia_mode, NULL);
        if (host_err < 0)
                goto out_nfserr;
+       if (created)
+               *created = 1;

        if (EX_ISSYNC(fhp->fh_export)) {
                err = nfserrno(nfsd_sync_dir(dentry));
diff --git a/include/linux/nfsd/nfsd.h b/include/linux/nfsd/nfsd.h
index eb23114..edb54c3 100644
--- a/include/linux/nfsd/nfsd.h
+++ b/include/linux/nfsd/nfsd.h
@@ -89,7 +89,7 @@ __be32                nfsd_access(struct svc_rqst *, s
 __be32         nfsd_create_v3(struct svc_rqst *, struct svc_fh *,
                                char *name, int len, struct iattr *attrs,
                                struct svc_fh *res, int createmode,
-                               u32 *verifier, int *truncp);
+                               u32 *verifier, int *truncp, int *created);
 __be32         nfsd_commit(struct svc_rqst *, struct svc_fh *,
                                loff_t, unsigned long);
 #endif /* CONFIG_NFSD_V3 */
--
1.4.3.3.g01929

-- 
Daniel J Blueman

  reply	other threads:[~2006-11-06 16:48 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-11-06 12:03 Poor NFSv4 first impressions Daniel J Blueman
2006-11-06 16:07 ` Bill Davidsen
2006-11-06 16:17 ` J. Bruce Fields
2006-11-06 16:23   ` [PATCH 0/1] nfsd4: reindent do_open_lookup() J. Bruce Fields
2006-11-06 16:26     ` J. Bruce Fields
2006-11-06 16:24   ` [PATCH 2/2] nfsd4: fix open-create permissions J. Bruce Fields
2006-11-06 16:48     ` Daniel J Blueman [this message]
2006-11-08 13:33       ` Fwd: " Jeff Garzik
2006-11-08 18:04         ` Andrew Morton
2006-11-07  7:20 ` Poor NFSv4 first impressions Pavel Machek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6278d2220611060848n3585ebc5odbf39efd6a02ab2@mail.gmail.com \
    --to=daniel.blueman@gmail.com \
    --cc=bfields@fieldses.org \
    --cc=jeff@garzik.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=neilb@suse.de \
    --cc=nfsv4@linux-nfs.org \
    --cc=torvalds@osdl.org \
    --cc=trond.myklebust@fys.uio.no \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®