Re: [PATCH v3 15/17] nfsd: move v0 checking out of nfsd_check_obj_isreg()

Jeff Layton <[email protected]> Thu, 16 Jul 2026 08:31:25 -0400
Newsgroups gmane.linux.nfs
Message-ID <[email protected]>
On Mon, 2026-07-13 at 16:15 +1000, NeilBrown wrote:
> From: NeilBrown <neil-+NVA1uvv1dVBDLzU/[email protected]>
> 
> A future patch will use nfsd_check_obj_isreg() in a context where the
> protocol version is not easily available.  So move the version check out
> and put it at the end of do_open_lookup().
> 
> Also change to return errno error code and use nfserrno() to convert to
> nfs error codes.  Use -ELOOP for nfserr_symlink, which is an error
> indication a problem with symlinks.  -EFTYPE is a good match for
> nfserr_wrong_type.
> 
> Signed-off-by: NeilBrown <neil-+NVA1uvv1dVBDLzU/[email protected]>
> ---
>  fs/nfsd/nfs4proc.c | 29 ++++++++++++-----------------
>  fs/nfsd/vfs.c      |  4 +++-
>  2 files changed, 15 insertions(+), 18 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 0d1bcb12ecbc..ffeda7214d66 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -168,23 +168,17 @@ do_open_permission(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nfs
>  	return fh_verify(rqstp, current_fh, S_IFREG, accmode);
>  }
>  
> -static __be32 nfsd_check_obj_isreg(struct dentry *child, u32 minor_version)
> +static __be32 nfsd_check_obj_isreg(struct dentry *child)
>  {
>  	umode_t mode = d_inode(child)->i_mode;
>  
>  	if (S_ISREG(mode))
> -		return nfs_ok;
> +		return 0;
>  	if (S_ISDIR(mode))
> -		return nfserr_isdir;
> +		return -EISDIR;
>  	if (S_ISLNK(mode))
> -		return nfserr_symlink;
> -
> -	/* RFC 7530 - 16.16.6 */
> -	if (minor_version == 0)
> -		return nfserr_symlink;
> -	else
> -		return nfserr_wrong_type;
> -
> +		return -ELOOP;
> +	return -EFTYPE;
>  }
>  
>  static void nfsd4_set_open_owner_reply_cache(struct nfsd4_compound_state *cstate, struct nfsd4_open *open, struct svc_fh *resfh)
> @@ -212,8 +206,6 @@ static __be32
>  nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
>  		  struct svc_fh *resfhp, struct nfsd4_open *open)
>  {
> -	struct nfsd4_compoundres *resp = rqstp->rq_resp;
> -	struct nfsd4_compound_state *cstate = &resp->cstate;
>  	struct iattr *iap = &open->op_iattr;
>  	struct nfsd_attrs attrs = {
>  		.na_iattr	= iap,
> @@ -356,8 +348,8 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
>  		 * op_filp and consequently a valid ->f_path.dentry.
>  		 */
>  
> -		status = nfsd_check_obj_isreg(child, cstate->minorversion);
> -		if (status == nfs_ok) {
> +		status = nfserrno(nfsd_check_obj_isreg(child));
> +		if (!status) {
>  			open->op_filp = dentry_open(&path, oflags,
>  						    current_cred());
>  			if (IS_ERR(open->op_filp)) {
> @@ -536,8 +528,7 @@ do_open_lookup(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, stru
>  	}
>  	if (status)
>  		goto out;
> -	status = nfsd_check_obj_isreg((*resfh)->fh_dentry,
> -				      cstate->minorversion);
> +	status = nfserrno(nfsd_check_obj_isreg((*resfh)->fh_dentry));
>  	if (status)
>  		goto out;
>  
> @@ -549,6 +540,10 @@ do_open_lookup(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, stru
>  	status = do_open_permission(rqstp, *resfh, open, accmode);
>  	set_change_info(&open->op_cinfo, current_fh);
>  out:
> +	if (status == nfserr_wrong_type && cstate->minorversion == 0)
> +		/* RFC 7530 - 16.16.6 */
> +		return nfserr_symlink;
> +
>  	return status;
>  }
>  
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index 9e05c3949cc1..c0e8c87a5e00 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -61,7 +61,7 @@ u64 nfsd_io_cache_write __read_mostly = NFSD_IO_BUFFERED;
>   * it's an error we don't expect, log it once and return nfserr_io.
>   */
>  __be32
> -nfserrno (int errno)
> +nfserrno(int errno)
>  {
>  	static struct {
>  		__be32	nfserr;
> @@ -105,6 +105,8 @@ nfserrno (int errno)
>  		{ nfserr_perm, -ENOKEY },
>  		{ nfserr_no_grace, -ENOGRACE},
>  		{ nfserr_io, -EBADMSG },
> +		{ nfserr_symlink, -ELOOP },
> +		{ nfserr_wrong_type, -EFTYPE },
>  	};
>  	int	i;
>  

Reviewed-by: Jeff Layton <[email protected]>