On Mon, Mar 04, 2019 at 02:08:22PM +1100, NeilBrown wrote:
> (Note that the commit hash in the Fixes tag is from the 'history'
>  tree - this bug predates git).
> Fixes: eb229d253e6c ("[PATCH] kNFSd: fix two xdr-encode bugs for readdirplus 
> reply")

It'd be nice to provide a URL for that.  The one I originally cloned one
seems to have disappeared.

> Cc: sta...@vger.kernel.org (v2.6.12+)
> Signed-off-by: NeilBrown <ne...@suse.com>
> ---
> 
> Can I still get extra credit for fixing a bug that is 14.5 years old, if
> I'm the one who introduced it?

Good grief, yes!  Great fix.  Is that a record?

And how did it go undetected so long, and what caused it to surface just
now?

I once thought about converting this over to the xdr_stream api that
NFSv4 uses to hide the page-crossing logic now.  But I think it's better
to leave it alone.

--b.

> 
>  fs/nfsd/nfs3proc.c | 16 ++++++++++++++--
>  fs/nfsd/nfs3xdr.c  |  1 +
>  2 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c
> index 9eb8086ea841..c9cf46e0c040 100644
> --- a/fs/nfsd/nfs3proc.c
> +++ b/fs/nfsd/nfs3proc.c
> @@ -463,8 +463,19 @@ nfsd3_proc_readdir(struct svc_rqst *rqstp)
>                                       &resp->common, nfs3svc_encode_entry);
>       memcpy(resp->verf, argp->verf, 8);
>       resp->count = resp->buffer - argp->buffer;
> -     if (resp->offset)
> -             xdr_encode_hyper(resp->offset, argp->cookie);
> +     if (resp->offset) {
> +             loff_t offset = argp->cookie;
> +
> +             if (unlikely(resp->offset1)) {
> +                     /* we ended up with offset on a page boundary */
> +                     *resp->offset = htonl(offset >> 32);
> +                     *resp->offset1 = htonl(offset & 0xffffffff);
> +                     resp->offset1 = NULL;
> +             } else {
> +                     xdr_encode_hyper(resp->offset, offset);
> +             }
> +             resp->offset = NULL;
> +     }
>  
>       RETURN_STATUS(nfserr);
>  }
> @@ -533,6 +544,7 @@ nfsd3_proc_readdirplus(struct svc_rqst *rqstp)
>               } else {
>                       xdr_encode_hyper(resp->offset, offset);
>               }
> +             resp->offset = NULL;
>       }
>  
>       RETURN_STATUS(nfserr);
> diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c
> index 9b973f4f7d01..83919116d5cb 100644
> --- a/fs/nfsd/nfs3xdr.c
> +++ b/fs/nfsd/nfs3xdr.c
> @@ -921,6 +921,7 @@ encode_entry(struct readdir_cd *ccd, const char *name, 
> int namlen,
>               } else {
>                       xdr_encode_hyper(cd->offset, offset64);
>               }
> +             cd->offset = NULL;
>       }
>  
>       /*
> -- 
> 2.14.0.rc0.dirty
> 


Reply via email to