Skip to content

Commit

Permalink
NFS: fix client permission error after adding user to permissible group
Browse files Browse the repository at this point in the history
The access cache only expires if either NFS_INO_INVALID_ACCESS flag is on
or timeout (without delegation).
Adding a user to a group in the NFS server will not cause any file
attributes to change.
The client will encounter permission errors until other file attributes
are changed or the memory cache is dropped.

Steps to reproduce the issue:
1.[client side] testuser is not part of testgroup
  testuser@kinetic:~$ ls -ld /mnt/private/
  drwxrwx--- 2 root testgroup 4096 Nov 24 08:23 /mnt/private/
  testuser@kinetic:~$ mktemp -p /mnt/private/
  mktemp: failed to create file via template
  ‘/mnt/private/tmp.XXXXXXXXXX’: Permission denied
2.[server side] add testuser into testgroup, which has access to folder
  root@kinetic:~$ usermod -aG testgroup testuser &&
  echo `date +'%s'` > /proc/net/rpc/auth.unix.gid/flush
3.[client side] create a file again but still fail
  testuser@kinetic:~$ mktemp -p /mnt/private/
  mktemp: failed to create file via template
  ‘/mnt/private/tmp.XXXXXXXXXX’: Permission denied

Signed-off-by: Chengen Du <chengen.du@canonical.com>
  • Loading branch information
yukariatlas authored and intel-lab-lkp committed Dec 13, 2022
1 parent 7fd461c commit 7206036
Show file tree
Hide file tree
Showing 9 changed files with 70 additions and 4 deletions.
2 changes: 1 addition & 1 deletion fs/nfs/dir.c
Expand Up @@ -2934,7 +2934,7 @@ static int access_cmp(const struct cred *a, const struct nfs_access_entry *b)
return 0;
}

static struct nfs_access_entry *nfs_access_search_rbtree(struct inode *inode, const struct cred *cred)
struct nfs_access_entry *nfs_access_search_rbtree(struct inode *inode, const struct cred *cred)
{
struct rb_node *n = NFS_I(inode)->access_cache.rb_node;

Expand Down
12 changes: 12 additions & 0 deletions fs/nfs/inode.c
Expand Up @@ -2178,6 +2178,18 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
nfsi->cache_validity |=
save_cache_validity & NFS_INO_INVALID_NLINK;

if (fattr->valid & NFS_ATTR_FATTR_ACCESS) {
if (!(invalid & NFS_INO_INVALID_ACCESS)) {
const struct cred *cred = current_cred();
struct nfs_access_entry *cache;

cache = nfs_access_search_rbtree(inode, cred);
if (cache != NULL && cache->mask != fattr->access) {
invalid |= NFS_INO_INVALID_ACCESS;
}
}
}

if (fattr->valid & NFS_ATTR_FATTR_SPACE_USED) {
/*
* report the blocks in 512byte units
Expand Down
3 changes: 3 additions & 0 deletions fs/nfs/nfs4_fs.h
Expand Up @@ -273,6 +273,9 @@ extern const struct dentry_operations nfs4_dentry_operations;
int nfs_atomic_open(struct inode *, struct dentry *, struct file *,
unsigned, umode_t);

extern struct nfs_access_entry *nfs_access_search_rbtree(struct inode *inode,
const struct cred *cred);

/* fs_context.c */
extern struct file_system_type nfs4_fs_type;

Expand Down
5 changes: 3 additions & 2 deletions fs/nfs/nfs4proc.c
Expand Up @@ -211,7 +211,8 @@ const u32 nfs4_fattr_bitmap[3] = {
| FATTR4_WORD1_TIME_ACCESS
| FATTR4_WORD1_TIME_METADATA
| FATTR4_WORD1_TIME_MODIFY
| FATTR4_WORD1_MOUNTED_ON_FILEID,
| FATTR4_WORD1_MOUNTED_ON_FILEID
| FATTR4_WORD1_ACCESS,
#ifdef CONFIG_NFS_V4_SECURITY_LABEL
FATTR4_WORD2_SECURITY_LABEL
#endif
Expand Down Expand Up @@ -3824,7 +3825,7 @@ static void nfs4_close_context(struct nfs_open_context *ctx, int is_sync)
nfs4_close_state(ctx->state, _nfs4_ctx_to_openmode(ctx));
}

#define FATTR4_WORD1_NFS40_MASK (2*FATTR4_WORD1_MOUNTED_ON_FILEID - 1UL)
#define FATTR4_WORD1_NFS40_MASK (2*FATTR4_WORD1_ACCESS - 1UL)
#define FATTR4_WORD2_NFS41_MASK (2*FATTR4_WORD2_SUPPATTR_EXCLCREAT - 1UL)
#define FATTR4_WORD2_NFS42_MASK (2*FATTR4_WORD2_XATTR_SUPPORT - 1UL)

Expand Down
25 changes: 25 additions & 0 deletions fs/nfs/nfs4xdr.c
Expand Up @@ -4294,6 +4294,26 @@ static int decode_attr_xattrsupport(struct xdr_stream *xdr, uint32_t *bitmap,
return 0;
}

static int decode_attr_access(struct xdr_stream *xdr, uint32_t *bitmap, uint32_t *access)
{
__be32 *p;
int ret = 0;

*access = 0;
if (unlikely(bitmap[1] & (FATTR4_WORD1_ACCESS - 1U)))
return -EIO;
if (likely(bitmap[1] & FATTR4_WORD1_ACCESS)) {
p = xdr_inline_decode(xdr, 4);
if (unlikely(!p))
return -EIO;
*access = be32_to_cpup(p);
bitmap[1] &= ~FATTR4_WORD1_ACCESS;
ret = NFS_ATTR_FATTR_ACCESS;
}
dprintk("%s: access=%u\n", __func__, (unsigned int)*access);
return ret;
}

static int verify_attr_len(struct xdr_stream *xdr, unsigned int savep, uint32_t attrlen)
{
unsigned int attrwords = XDR_QUADLEN(attrlen);
Expand Down Expand Up @@ -4745,6 +4765,11 @@ static int decode_getfattr_attrs(struct xdr_stream *xdr, uint32_t *bitmap,
goto xdr_error;
fattr->valid |= status;

status = decode_attr_access(xdr, bitmap, &fattr->access);
if (status < 0)
goto xdr_error;
fattr->valid |= status;

status = -EIO;
if (unlikely(bitmap[1]))
goto xdr_error;
Expand Down
21 changes: 21 additions & 0 deletions fs/nfsd/nfs4xdr.c
Expand Up @@ -3337,6 +3337,27 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp,
}
p = xdr_encode_hyper(p, ino);
}
if (bmval1 & FATTR4_WORD1_ACCESS) {
u32 access;
u32 supported;

access = NFS3_ACCESS_READ | NFS3_ACCESS_MODIFY | NFS3_ACCESS_EXTEND;
if (minorversion >= 2) {
access |= NFS4_ACCESS_XALIST | NFS4_ACCESS_XAREAD |
NFS4_ACCESS_XAWRITE;
}
if (S_ISDIR(d_inode(dentry)->i_mode))
access |= NFS3_ACCESS_DELETE | NFS3_ACCESS_LOOKUP;
else
access |= NFS3_ACCESS_EXECUTE;
status = nfsd_access(rqstp, fhp, &access, &supported);
if (status)
goto out;
p = xdr_reserve_space(xdr, 4);
if (!p)
goto out_resource;
*p++ = cpu_to_be32(access);
}
#ifdef CONFIG_NFSD_PNFS
if (bmval1 & FATTR4_WORD1_FS_LAYOUT_TYPES) {
status = nfsd4_encode_layout_types(xdr, exp->ex_layout_types);
Expand Down
3 changes: 2 additions & 1 deletion fs/nfsd/nfsd.h
Expand Up @@ -379,7 +379,8 @@ void nfsd_lockd_shutdown(void);
| FATTR4_WORD1_SPACE_AVAIL | FATTR4_WORD1_SPACE_FREE | FATTR4_WORD1_SPACE_TOTAL \
| FATTR4_WORD1_SPACE_USED | FATTR4_WORD1_TIME_ACCESS | FATTR4_WORD1_TIME_ACCESS_SET \
| FATTR4_WORD1_TIME_DELTA | FATTR4_WORD1_TIME_METADATA | FATTR4_WORD1_TIME_CREATE \
| FATTR4_WORD1_TIME_MODIFY | FATTR4_WORD1_TIME_MODIFY_SET | FATTR4_WORD1_MOUNTED_ON_FILEID)
| FATTR4_WORD1_TIME_MODIFY | FATTR4_WORD1_TIME_MODIFY_SET | FATTR4_WORD1_MOUNTED_ON_FILEID \
| FATTR4_WORD1_ACCESS)

#define NFSD4_SUPPORTED_ATTRS_WORD2 0

Expand Down
1 change: 1 addition & 0 deletions include/linux/nfs4.h
Expand Up @@ -451,6 +451,7 @@ enum lock_type4 {
#define FATTR4_WORD1_TIME_MODIFY (1UL << 21)
#define FATTR4_WORD1_TIME_MODIFY_SET (1UL << 22)
#define FATTR4_WORD1_MOUNTED_ON_FILEID (1UL << 23)
#define FATTR4_WORD1_ACCESS (1UL << 24)
#define FATTR4_WORD1_DACL (1UL << 26)
#define FATTR4_WORD1_SACL (1UL << 27)
#define FATTR4_WORD1_FS_LAYOUT_TYPES (1UL << 30)
Expand Down
2 changes: 2 additions & 0 deletions include/linux/nfs_xdr.h
Expand Up @@ -78,6 +78,7 @@ struct nfs_fattr {
struct nfs4_string *group_name;
struct nfs4_threshold *mdsthreshold; /* pNFS threshold hints */
struct nfs4_label *label;
__u32 access;
};

#define NFS_ATTR_FATTR_TYPE (1U << 0)
Expand Down Expand Up @@ -106,6 +107,7 @@ struct nfs_fattr {
#define NFS_ATTR_FATTR_OWNER_NAME (1U << 23)
#define NFS_ATTR_FATTR_GROUP_NAME (1U << 24)
#define NFS_ATTR_FATTR_V4_SECURITY_LABEL (1U << 25)
#define NFS_ATTR_FATTR_ACCESS (1U << 26)

#define NFS_ATTR_FATTR (NFS_ATTR_FATTR_TYPE \
| NFS_ATTR_FATTR_MODE \
Expand Down

0 comments on commit 7206036

Please sign in to comment.