linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "J. Bruce Fields" <bfields@fieldses.org>
To: Steve Dickson <SteveD@redhat.com>
Cc: Rishi Agrawal <Rishi_Agrawal@symantec.com>,
	"linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>,
	Rajesh Ghanekar <Rajesh_Ghanekar@symantec.com>,
	Ram Pandiri <ram_pandiri@symantec.com>,
	Sreeharsha Sarabu <Sreeharsha_Sarabu@symantec.com>,
	Abhijit Dey <Abhijit_Dey@symantec.com>,
	Tushar Shinde <Tushar_Shinde@symantec.com>,
	"bfields@redhat.com" <bfields@redhat.com>
Subject: Re: [PATCH] nfsd: allow turning off nfsv3 readdir_plus
Date: Tue, 5 Aug 2014 14:21:34 -0400	[thread overview]
Message-ID: <20140805182134.GQ23341@fieldses.org> (raw)
In-Reply-To: <20140804214646.GK23341@fieldses.org>

On Mon, Aug 04, 2014 at 05:46:47PM -0400, J. Bruce Fields wrote:
> On Mon, Aug 04, 2014 at 11:24:11AM -0400, bfields wrote:
> > +static int
> > +nfsd3_is_readdirplus_supported(struct svc_rqst *rqstp, struct svc_fh *fhp)
> > +{
> > +	struct svc_export *exp;
> > +	int supported = 1; /* fall back to readdirplus supported in case of errors.*/
> > +	int err;
> > +
> > +	err = fh_verify(rqstp, fhp, S_IFDIR, NFSD_MAY_READ);
> > +	if (err) {
> > +		goto out;
> > +	}
> 
> Actually, this isn't right: errors from fh_verify should be returned to
> the client or weird things could happen (e.g. what should have been a
> transient DELAY error could result in the client turning off
> readdirplus).

Apologies, I misread: as the comment above notes, it falls back on
allowing readdirplus when this fails, so I don't think there's a real
bug here.

> And MAY_READ is more than nfsd_readdir actually asks for,
> I think, probably should just be MAY_NOP here.
> 
> I'll fix that up.--b.

But it's probably still better to return the fh_verify error on failure,
as follows.

--b.

diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
index 72ffd7cce3c3..30a739d896ff 100644
--- a/fs/nfsd/export.c
+++ b/fs/nfsd/export.c
@@ -1145,6 +1145,7 @@ static struct flags {
 	{ NFSEXP_ALLSQUASH, {"all_squash", ""}},
 	{ NFSEXP_ASYNC, {"async", "sync"}},
 	{ NFSEXP_GATHERED_WRITES, {"wdelay", "no_wdelay"}},
+	{ NFSEXP_NOREADDIRPLUS, {"nordirplus", ""}},
 	{ NFSEXP_NOHIDE, {"nohide", ""}},
 	{ NFSEXP_CROSSMOUNT, {"crossmnt", ""}},
 	{ NFSEXP_NOSUBTREECHECK, {"no_subtree_check", ""}},
diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c
index fa2525b2e9d7..247b06fb400d 100644
--- a/fs/nfsd/nfs3proc.c
+++ b/fs/nfsd/nfs3proc.c
@@ -471,6 +471,14 @@ nfsd3_proc_readdirplus(struct svc_rqst *rqstp, struct nfsd3_readdirargs *argp,
 	resp->buflen = resp->count;
 	resp->rqstp = rqstp;
 	offset = argp->cookie;
+
+	nfserr = fh_verify(rqstp, &resp->fh, S_IFDIR, NFSD_MAY_NOP);
+	if (nfserr)
+		RETURN_STATUS(nfserr);
+
+	if (resp->fh.fh_export->ex_flags & NFSEXP_NOREADDIRPLUS)
+		RETURN_STATUS(nfserr_notsupp);
+
 	nfserr = nfsd_readdir(rqstp, &resp->fh,
 				     &offset,
 				     &resp->common,
diff --git a/include/uapi/linux/nfsd/export.h b/include/uapi/linux/nfsd/export.h
index cf47c313794e..584b6ef3a5e8 100644
--- a/include/uapi/linux/nfsd/export.h
+++ b/include/uapi/linux/nfsd/export.h
@@ -28,7 +28,8 @@
 #define NFSEXP_ALLSQUASH	0x0008
 #define NFSEXP_ASYNC		0x0010
 #define NFSEXP_GATHERED_WRITES	0x0020
-/* 40 80 100 currently unused */
+#define NFSEXP_NOREADDIRPLUS    0x0040
+/* 80 100 currently unused */
 #define NFSEXP_NOHIDE		0x0200
 #define NFSEXP_NOSUBTREECHECK	0x0400
 #define	NFSEXP_NOAUTHNLM	0x0800		/* Don't authenticate NLM requests - just trust */
@@ -47,7 +48,7 @@
  */
 #define	NFSEXP_V4ROOT		0x10000
 /* All flags that we claim to support.  (Note we don't support NOACL.) */
-#define NFSEXP_ALLFLAGS		0x17E3F
+#define NFSEXP_ALLFLAGS		0x1FE7F
 
 /* The flags that may vary depending on security flavor: */
 #define NFSEXP_SECINFO_FLAGS	(NFSEXP_READONLY | NFSEXP_ROOTSQUASH \

  reply	other threads:[~2014-08-05 18:21 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-25 16:19 Patch For Making Readdir_plus configurable Rishi Agrawal
2014-07-25 16:54 ` Christopher T Vogan
     [not found] ` <OF57CEB932.B84FFC9B-ON87257D20.005C9233-86257D20.005CC28D@us.ibm.com>
2014-07-28  3:17   ` Rishi Agrawal
2014-07-29 20:34     ` J. Bruce Fields
2014-08-04 14:31 ` Steve Dickson
2014-08-04 15:24   ` [PATCH] nfsd: allow turning off nfsv3 readdir_plus J. Bruce Fields
2014-08-04 21:46     ` J. Bruce Fields
2014-08-05 18:21       ` J. Bruce Fields [this message]
2014-08-18 17:47         ` Rajesh Ghanekar
2014-08-18 18:06           ` Rajesh Ghanekar
2014-08-18 19:10             ` J. Bruce Fields
2014-08-18 21:19             ` J. Bruce Fields
2014-08-18 21:42               ` Abhijit Dey
2014-08-19  7:53                 ` Rajesh Ghanekar
2014-08-05  6:58   ` Patch For Making Readdir_plus configurable Rishi Agrawal

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=20140805182134.GQ23341@fieldses.org \
    --to=bfields@fieldses.org \
    --cc=Abhijit_Dey@symantec.com \
    --cc=Rajesh_Ghanekar@symantec.com \
    --cc=Rishi_Agrawal@symantec.com \
    --cc=Sreeharsha_Sarabu@symantec.com \
    --cc=SteveD@redhat.com \
    --cc=Tushar_Shinde@symantec.com \
    --cc=bfields@redhat.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=ram_pandiri@symantec.com \
    /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;
as well as URLs for NNTP newsgroup(s).