Re: [RFC PATCH] NFSD: Fix READDIR on NFSv3 mounts of ext4 exports

[Jeff Layton <jlayton@kernel.org>]

On Sun, 2024-11-03 at 16:37 -0500, cel@kernel.org wrote:
> From: Chuck Lever <chuck.lever@oracle.com>
> 
> I noticed that recently, simple operations like "make" started
> failing on NFSv3 mounts of ext4 exports. Network capture shows that
> READDIRPLUS operated correctly but READDIR failed with
> NFS3ERR_INVAL. The vfs_llseek() call returned EINVAL when it is
> passed a non-zero starting directory cookie.
> 
> I bisected to commit c689bdd3bffa ("nfsd: further centralize
> protocol version checks.").
> 
> Turns out that nfsd3_proc_readdir() does not call fh_verify() before
> it calls nfsd_readdir(), so the new fhp->fh_64bit_cookies boolean is
> not set properly. This leaves the NFSD_MAY_64BIT_COOKIE unset when
> the directory is opened.
> 
> For ext4, this causes the wrong "max file size" value to be used
> when sanity checking the incoming directory cookie (which is a seek
> offset value).
> 
> Both NFSv2 and NFSv3 READDIR need to call fh_verify() now to ensure
> the new boolean fields are properly initialized.
> 
> There is a risk that these procedures might now return a status code
> that is not valid (by spec), or that operations that are currently
> allowed might no longer be.
> 
> Fixes: c689bdd3bffa ("nfsd: further centralize protocol version checks.")
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  fs/nfsd/nfs3proc.c | 6 ++++++
>  fs/nfsd/nfsproc.c  | 8 +++++++-
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c
> index dfcc957e460d..48bcdc96b867 100644
> --- a/fs/nfsd/nfs3proc.c
> +++ b/fs/nfsd/nfs3proc.c
> @@ -592,6 +592,11 @@ nfsd3_proc_readdir(struct svc_rqst *rqstp)
>  	resp->cookie_offset = 0;
>  	resp->rqstp = rqstp;
>  	offset = argp->cookie;
> +
> +	resp->status = fh_verify(rqstp, &resp->fh, S_IFDIR, NFSD_MAY_NOP);
> +	if (resp->status != nfs_ok)
> +		goto out;
> +
>  	resp->status = nfsd_readdir(rqstp, &resp->fh, &offset,
>  				    &resp->common, nfs3svc_encode_entry3);
>  	memcpy(resp->verf, argp->verf, 8);
> @@ -600,6 +605,7 @@ nfsd3_proc_readdir(struct svc_rqst *rqstp)
>  	/* Recycle only pages that were part of the reply */
>  	rqstp->rq_next_page = resp->xdr.page_ptr + 1;
>  
> +out:
>  	return rpc_success;
>  }
>  
> diff --git a/fs/nfsd/nfsproc.c b/fs/nfsd/nfsproc.c
> index 97aab34593ef..ebe8fd3c9ddd 100644
> --- a/fs/nfsd/nfsproc.c
> +++ b/fs/nfsd/nfsproc.c
> @@ -586,11 +586,17 @@ nfsd_proc_readdir(struct svc_rqst *rqstp)
>  	resp->common.err = nfs_ok;
>  	resp->cookie_offset = 0;
>  	offset = argp->cookie;
> +
> +	resp->status = fh_verify(rqstp, &resp->fh, S_IFDIR, NFSD_MAY_NOP);
> +	if (resp->status != nfs_ok)
> +		goto out;
> +
>  	resp->status = nfsd_readdir(rqstp, &argp->fh, &offset,
>  				    &resp->common, nfssvc_encode_entry);
>  	nfssvc_encode_nfscookie(resp, offset);
> -
>  	fh_put(&argp->fh);
> +
> +out:
>  	return rpc_success;
>  }
>  

Reviewed-by: Jeff Layton <jlayton@kernel.org>