nfs_access_get_cached() reports (on success) the access modes known to be permitted. The caller than has to compare this against the wanted modes. This is replaced with nfs_access_check_cached() which is told what modes are wanted and returns -EACCES if they are not available, 0 if they are, or -ECHILD or -ENOENT as before. This new interface will allow more subtlety for choose when to trust the cache to be encoded in nfs_access_check_cached(), which is then available to all callers. nfs_access_calc_mask() is changed to nfs_calc_access_mask(). This new function is given a set of Linux MAY_* flags and returns the corresponding set of NFS_ACCESS_* flags. This is the inverse of nfs_access_calc_mask(). Because of this inversion there is a small change in behaviour. For a non-file, non-directory, the previous code would grant MAY_WRITE if any for NFS_ACCESS_MODIFY, NFS_ACCESS_EXTEND, NFS_ACCESS_DELETE were granted. This new code does not support this "any" approach, and it isn't clear it is the most correct approach. So for non-file, non-directories, MAY_WRITE will only be granted if NFS_ACCESS_MODIFY is granted. The nfs_access_exit trace event now reports the NFS_ACCESS flags, not he Linux MAY flags. Signed-off-by: NeilBrown <neilb@xxxxxxx> --- fs/nfs/dir.c | 62 +++++++++++++++++++++++++----------------------- fs/nfs/nfs4proc.c | 24 +++++++------------ include/linux/nfs_fs.h | 4 ++- 3 files changed, 43 insertions(+), 47 deletions(-) diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c index c6b263b5faf1..b4e2c6a34234 100644 --- a/fs/nfs/dir.c +++ b/fs/nfs/dir.c @@ -2971,19 +2971,22 @@ static int nfs_access_get_cached_rcu(struct inode *inode, const struct cred *cre return err; } -int nfs_access_get_cached(struct inode *inode, const struct cred *cred, - u32 *mask, bool may_block) +int nfs_access_check_cached(struct inode *inode, const struct cred *cred, + u32 want, bool may_block) { + u32 have; int status; - status = nfs_access_get_cached_rcu(inode, cred, mask); + status = nfs_access_get_cached_rcu(inode, cred, &have); if (status != 0) - status = nfs_access_get_cached_locked(inode, cred, mask, - may_block); + status = nfs_access_get_cached_locked(inode, cred, &have, + may_block); + if (status == 0 && (want & ~have) != 0) + status = -EACCES; return status; } -EXPORT_SYMBOL_GPL(nfs_access_get_cached); +EXPORT_SYMBOL_GPL(nfs_access_check_cached); static void nfs_access_add_rbtree(struct inode *inode, struct nfs_access_entry *set, @@ -3067,26 +3070,26 @@ EXPORT_SYMBOL_GPL(nfs_access_add_cache); #define NFS_DIR_MAY_WRITE NFS_MAY_WRITE #define NFS_MAY_LOOKUP (NFS_ACCESS_LOOKUP) #define NFS_MAY_EXECUTE (NFS_ACCESS_EXECUTE) -static int -nfs_access_calc_mask(u32 access_result, umode_t umode) +static u32 +nfs_calc_access_mask(int may_mask, umode_t umode) { - int mask = 0; + int access_mask = 0; - if (access_result & NFS_MAY_READ) - mask |= MAY_READ; + if (may_mask & MAY_READ) + access_mask |= NFS_MAY_READ; if (S_ISDIR(umode)) { - if ((access_result & NFS_DIR_MAY_WRITE) == NFS_DIR_MAY_WRITE) - mask |= MAY_WRITE; - if ((access_result & NFS_MAY_LOOKUP) == NFS_MAY_LOOKUP) - mask |= MAY_EXEC; + if (may_mask & MAY_WRITE) + access_mask |= NFS_DIR_MAY_WRITE; + if (may_mask & MAY_EXEC) + access_mask |= NFS_MAY_LOOKUP; } else if (S_ISREG(umode)) { - if ((access_result & NFS_FILE_MAY_WRITE) == NFS_FILE_MAY_WRITE) - mask |= MAY_WRITE; - if ((access_result & NFS_MAY_EXECUTE) == NFS_MAY_EXECUTE) - mask |= MAY_EXEC; - } else if (access_result & NFS_MAY_WRITE) - mask |= MAY_WRITE; - return mask; + if (may_mask & MAY_WRITE) + access_mask |= NFS_FILE_MAY_WRITE; + if (may_mask & MAY_EXEC) + access_mask |= NFS_MAY_EXECUTE; + } else if (may_mask & MAY_WRITE) + access_mask |= NFS_ACCESS_MODIFY; + return access_mask; } void nfs_access_set_mask(struct nfs_access_entry *entry, u32 access_result) @@ -3099,14 +3102,15 @@ static int nfs_do_access(struct inode *inode, const struct cred *cred, int mask) { struct nfs_access_entry cache; bool may_block = (mask & MAY_NOT_BLOCK) == 0; - int cache_mask = -1; + u32 want; int status; trace_nfs_access_enter(inode); - status = nfs_access_get_cached(inode, cred, &cache.mask, may_block); - if (status == 0) - goto out_cached; + want = nfs_calc_access_mask(mask, inode->i_mode); + status = nfs_access_check_cached(inode, cred, want, may_block); + if (status == 0 || status == -EACCES) + goto out; status = -ECHILD; if (!may_block) @@ -3132,12 +3136,10 @@ static int nfs_do_access(struct inode *inode, const struct cred *cred, int mask) goto out; } nfs_access_add_cache(inode, &cache, cred); -out_cached: - cache_mask = nfs_access_calc_mask(cache.mask, inode->i_mode); - if ((mask & ~cache_mask & (MAY_READ | MAY_WRITE | MAY_EXEC)) != 0) + if ((want & ~cache.mask) != 0) status = -EACCES; out: - trace_nfs_access_exit(inode, mask, cache_mask, status); + trace_nfs_access_exit(inode, want, cache.mask, status); return status; } diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 16106f805ffa..1e80d88df588 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -7719,7 +7719,6 @@ static int nfs4_xattr_set_nfs4_user(const struct xattr_handler *handler, const char *key, const void *buf, size_t buflen, int flags) { - u32 mask; int ret; if (!nfs_server_capable(inode, NFS_CAP_XATTR)) @@ -7734,10 +7733,9 @@ static int nfs4_xattr_set_nfs4_user(const struct xattr_handler *handler, * do a cached access check for the XA* flags to possibly avoid * doing an RPC and getting EACCES back. */ - if (!nfs_access_get_cached(inode, current_cred(), &mask, true)) { - if (!(mask & NFS_ACCESS_XAWRITE)) - return -EACCES; - } + if (nfs_access_check_cached(inode, current_cred(), NFS_ACCESS_XAWRITE, + true) == -EACCES) + return -EACCES; if (buf == NULL) { ret = nfs42_proc_removexattr(inode, key); @@ -7756,16 +7754,14 @@ static int nfs4_xattr_get_nfs4_user(const struct xattr_handler *handler, struct dentry *unused, struct inode *inode, const char *key, void *buf, size_t buflen) { - u32 mask; ssize_t ret; if (!nfs_server_capable(inode, NFS_CAP_XATTR)) return -EOPNOTSUPP; - if (!nfs_access_get_cached(inode, current_cred(), &mask, true)) { - if (!(mask & NFS_ACCESS_XAREAD)) - return -EACCES; - } + if (nfs_access_check_cached(inode, current_cred(), NFS_ACCESS_XAREAD, + true) == -EACCES) + return -EACCES; ret = nfs_revalidate_inode(inode, NFS_INO_INVALID_CHANGE); if (ret) @@ -7788,15 +7784,13 @@ nfs4_listxattr_nfs4_user(struct inode *inode, char *list, size_t list_len) ssize_t ret, size; char *buf; size_t buflen; - u32 mask; if (!nfs_server_capable(inode, NFS_CAP_XATTR)) return 0; - if (!nfs_access_get_cached(inode, current_cred(), &mask, true)) { - if (!(mask & NFS_ACCESS_XALIST)) - return 0; - } + if (nfs_access_check_cached(inode, current_cred(), NFS_ACCESS_XALIST, + true) == -EACCES) + return 0; ret = nfs_revalidate_inode(inode, NFS_INO_INVALID_CHANGE); if (ret) diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h index b48b9259e02c..7614f621c42d 100644 --- a/include/linux/nfs_fs.h +++ b/include/linux/nfs_fs.h @@ -530,8 +530,8 @@ extern int nfs_instantiate(struct dentry *dentry, struct nfs_fh *fh, struct nfs_fattr *fattr); extern int nfs_may_open(struct inode *inode, const struct cred *cred, int openflags); extern void nfs_access_zap_cache(struct inode *inode); -extern int nfs_access_get_cached(struct inode *inode, const struct cred *cred, - u32 *mask, bool may_block); +extern int nfs_access_check_cached(struct inode *inode, const struct cred *cred, + u32 want, bool may_block); /* * linux/fs/nfs/symlink.c