On Fri, Aug 29, 2014 at 03:29:07PM -0400, Trond Myklebust wrote: > On Fri, Aug 29, 2014 at 2:55 PM, J. Bruce Fields <bfields@xxxxxxxxxxxx> wrote: > > From: "J. Bruce Fields" <bfields@xxxxxxxxxx> > > > > Commit 3b299709091b "nfsd4: enforce rd_dircount" totally misunderstood > > rd_dircount; it refers to total non-attribute bytes returned, not number > > of directory entries returned. > > > > Bring the code into agreement with RFC 3530 section 14.2.24. > > > > Cc: stable@xxxxxxxxxx > > Signed-off-by: J. Bruce Fields <bfields@xxxxxxxxxx> > > --- > > fs/nfsd/nfs4xdr.c | 14 +++++++++++++- > > 1 file changed, 13 insertions(+), 1 deletion(-) > > > > diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c > > index f9821ce6658a..e94457c33ad6 100644 > > --- a/fs/nfsd/nfs4xdr.c > > +++ b/fs/nfsd/nfs4xdr.c > > @@ -2657,6 +2657,7 @@ nfsd4_encode_dirent(void *ccdv, const char *name, int namlen, > > struct xdr_stream *xdr = cd->xdr; > > int start_offset = xdr->buf->len; > > int cookie_offset; > > + u32 name_and_cookie; > > int entry_bytes; > > __be32 nfserr = nfserr_toosmall; > > __be64 wire_offset; > > @@ -2718,7 +2719,14 @@ nfsd4_encode_dirent(void *ccdv, const char *name, int namlen, > > cd->rd_maxcount -= entry_bytes; > > if (!cd->rd_dircount) > > goto fail; > > - cd->rd_dircount--; > > + /* > > + * RFC 3530 14.2.24 describes rd_dircount as only a "hint", so > > + * let's always let through the first entry, at least: > > + */ > > + name_and_cookie = 4 * XDR_QUADLEN(namlen) + 8; > > + if (name_and_cookie > cd->rd_dircount && cd->cookie_offset) > > + goto fail; > > + cd->rd_dircount -= min(cd->rd_dircount, name_and_cookie); > > cd->cookie_offset = cookie_offset; > > skip_entry: > > cd->common.err = nfs_ok; > > @@ -3321,6 +3329,10 @@ nfsd4_encode_readdir(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4 > > } > > maxcount = min_t(int, maxcount-16, bytes_left); > > > > + /* RFC 3530 14.2.24 allows us to ignore dircount when it's 0: */ > > + if (!readdir->rd_dircount) > > + readdir->rd_dircount = INT_MAX; > > + > > Ah... Time to change the Linux client to always set this value to 0. > It really is useless in our case. I didn't look at the client. Hm: uint32_t dircount = readdir->count >> 1 ... *p++ = cpu_to_be32(dircount); *p++ = cpu_to_be32(readdir->count); Yeah, that's kind of arbitrary. This dircount thing is just inherently weird. We could just revert the server back to ignoring it completely like it used to--the spec only says it's a "hint", whatever that means. But presumably somebody must have thought they had a use for it. --b. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
