Re: [PATCH] NFS: fix access bit translation in _nfs4_proc_access()

[Lucas Stach]

Am Freitag, den 10.11.2017, 16:03 +0100 schrieb Lucas Stach:
> Since bd8b2441742b (NFS: Store the raw NFS access mask in the inode's
> access cache) the mask already contains the NFS access bits, so it is
> wrong to try to map them from the MAY_* bits to the NFS_MAY_* bits
> in _nfs4_proc_access(). This leads to bogus access permissions being
> returned from the server.
> 
> Fix this by using the mask directly, but be careful to not ask for
> to broad permissions from the calling nfs_do_access().
> 
> CC: stable@xxxxxxxxxxxxxxx #4.13+
> Signed-off-by: Lucas Stach <l.stach@xxxxxxxxxxxxxx>
> Tested-by: BjÃrn LÃssig <b.laessig@xxxxxxxxxxxxxx>

This patch is incomplete, as it misses the NFSv3 parts. Please ignore
this patch.

Regards,
Lucas

> ---
> Âfs/nfs/dir.cÂÂÂÂÂÂ|ÂÂ8 ++++++--
> Âfs/nfs/nfs4proc.c | 19 +------------------
> Â2 files changed, 7 insertions(+), 20 deletions(-)
> 
> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> index 5ceaeb1f6fb6..dd3a06353bdc 100644
> --- a/fs/nfs/dir.c
> +++ b/fs/nfs/dir.c
> @@ -2426,8 +2426,12 @@ static int nfs_do_access(struct inode *inode,
> struct rpc_cred *cred, int mask)
> Â		goto out;
> Â
> Â	/* Be clever: ask server to check for all possible rights */
> -	cache.mask = NFS_MAY_LOOKUP | NFS_MAY_EXECUTE
> -		ÂÂÂÂÂ| NFS_MAY_WRITE | NFS_MAY_READ;
> +	cache.mask = NFS_MAY_READ;
> +	if (S_ISDIR(inode->i_mode))
> +		cache.mask |= NFS_DIR_MAY_WRITE | NFS_MAY_LOOKUP;
> +	else
> +		cache.mask |= NFS_FILE_MAY_WRITE | NFS_MAY_EXECUTE;
> +
> Â	cache.cred = cred;
> Â	status = NFS_PROTO(inode)->access(inode, &cache);
> Â	if (status != 0) {
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 6c61e2b99635..cb7b5aafaef6 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -3889,6 +3889,7 @@ static int _nfs4_proc_access(struct inode
> *inode, struct nfs_access_entry *entry
> Â	struct nfs4_accessargs args = {
> Â		.fh = NFS_FH(inode),
> Â		.bitmask = server->cache_consistency_bitmask,
> +		.access = entry->mask,
> Â	};
> Â	struct nfs4_accessres res = {
> Â		.server = server,
> @@ -3899,26 +3900,8 @@ static int _nfs4_proc_access(struct inode
> *inode, struct nfs_access_entry *entry
> Â		.rpc_resp = &res,
> Â		.rpc_cred = entry->cred,
> Â	};
> -	int mode = entry->mask;
> Â	int status = 0;
> Â
> -	/*
> -	Â* Determine which access bits we want to ask for...
> -	Â*/
> -	if (mode & MAY_READ)
> -		args.access |= NFS4_ACCESS_READ;
> -	if (S_ISDIR(inode->i_mode)) {
> -		if (mode & MAY_WRITE)
> -			args.access |= NFS4_ACCESS_MODIFY |
> NFS4_ACCESS_EXTEND | NFS4_ACCESS_DELETE;
> -		if (mode & MAY_EXEC)
> -			args.access |= NFS4_ACCESS_LOOKUP;
> -	} else {
> -		if (mode & MAY_WRITE)
> -			args.access |= NFS4_ACCESS_MODIFY |
> NFS4_ACCESS_EXTEND;
> -		if (mode & MAY_EXEC)
> -			args.access |= NFS4_ACCESS_EXECUTE;
> -	}
> -
> Â	res.fattr = nfs_alloc_fattr();
> Â	if (res.fattr == NULL)
> Â		return -ENOMEM;