From 76b9cf2565eb7a5dff65bf508d5762a109345f5d Mon Sep 17 00:00:00 2001 From: Andreas Dilger Date: Fri, 18 Oct 2019 20:12:24 +0900 Subject: [PATCH] LU-12885 mdd: clearly name variables for MAY_ flags Clearly name variables for MAY_READ, MAY_WRITE, MAY_EXEC to distinguish it from other "mask" variables. The kernel VFS silently converts the MAY_READ, MAY_WRITE, and MAY_EXEC flags to ACL_READ, ACL_WRITE, and ACL_EXECUTE modes for the on-disk ACLs. It later also converts from the ACL_* flags to POSIX rwx bits. Verify that these values are the same. Test-Parameters: trivial Signed-off-by: Andreas Dilger Change-Id: Idcd91d4c467c4415f1c67a5081721393cd3ebbe5 Reviewed-on: https://review.whamcloud.com/36520 Tested-by: jenkins Tested-by: Maloo Reviewed-by: James Simmons Reviewed-by: Arshad Hussain Reviewed-by: Neil Brown Reviewed-by: Oleg Drokin --- lustre/include/lustre_eacl.h | 3 ++- lustre/include/lustre_mds.h | 10 +++---- lustre/include/md_object.h | 6 ++--- lustre/mdd/mdd_dir.c | 4 +-- lustre/mdd/mdd_internal.h | 15 ++++++----- lustre/mdd/mdd_object.c | 13 +++++----- lustre/mdd/mdd_permission.c | 62 +++++++++++++++++++++++--------------------- lustre/obdclass/acl.c | 35 ++++++++++++++++++++----- 8 files changed, 89 insertions(+), 59 deletions(-) diff --git a/lustre/include/lustre_eacl.h b/lustre/include/lustre_eacl.h index 347a8dc..4f83a93 100644 --- a/lustre/include/lustre_eacl.h +++ b/lustre/include/lustre_eacl.h @@ -77,7 +77,8 @@ struct lustre_idmap_table; #endif extern int lustre_posix_acl_permission(struct lu_ucred *mu, - const struct lu_attr *la, int want, + const struct lu_attr *la, + unsigned int may_mask, posix_acl_xattr_entry *entry, int count); extern int lustre_posix_acl_chmod_masq(posix_acl_xattr_entry *entry, diff --git a/lustre/include/lustre_mds.h b/lustre/include/lustre_mds.h index 31e05f9..c545b63 100644 --- a/lustre/include/lustre_mds.h +++ b/lustre/include/lustre_mds.h @@ -68,16 +68,16 @@ static inline int md_should_create(u64 open_flags) /* do NOT or the MAY_*'s, you'll get the weakest */ static inline int mds_accmode(u64 open_flags) { - int res = 0; + unsigned int may_mask = 0; if (open_flags & MDS_FMODE_READ) - res |= MAY_READ; + may_mask |= MAY_READ; if (open_flags & (MDS_FMODE_WRITE | MDS_OPEN_TRUNC | MDS_OPEN_APPEND)) - res |= MAY_WRITE; + may_mask |= MAY_WRITE; if (open_flags & MDS_FMODE_EXEC) - res = MAY_EXEC; + may_mask = MAY_EXEC; - return res; + return may_mask; } /** @} mds */ diff --git a/lustre/include/md_object.h b/lustre/include/md_object.h index 3ab8a79..8767061 100644 --- a/lustre/include/md_object.h +++ b/lustre/include/md_object.h @@ -229,7 +229,7 @@ union ldlm_policy_data; struct md_object_operations { int (*moo_permission)(const struct lu_env *env, struct md_object *pobj, struct md_object *cobj, - struct md_attr *attr, int mask); + struct md_attr *attr, unsigned int may_mask); int (*moo_attr_get)(const struct lu_env *env, struct md_object *obj, struct md_attr *attr); @@ -424,10 +424,10 @@ static inline struct md_object *md_object_find_slice(const struct lu_env *env, /** md operations */ static inline int mo_permission(const struct lu_env *env, struct md_object *p, struct md_object *c, struct md_attr *at, - int mask) + unsigned int may_mask) { LASSERT(c->mo_ops->moo_permission); - return c->mo_ops->moo_permission(env, p, c, at, mask); + return c->mo_ops->moo_permission(env, p, c, at, may_mask); } static inline int mo_attr_get(const struct lu_env *env, struct md_object *m, diff --git a/lustre/mdd/mdd_dir.c b/lustre/mdd/mdd_dir.c index c0cbf55..6060b1f 100644 --- a/lustre/mdd/mdd_dir.c +++ b/lustre/mdd/mdd_dir.c @@ -70,7 +70,7 @@ mdd_name_check(struct mdd_device *m, const struct lu_name *ln) static int __mdd_lookup(const struct lu_env *env, struct md_object *pobj, const struct lu_attr *pattr, const struct lu_name *lname, - struct lu_fid* fid, int mask) + struct lu_fid *fid, unsigned int may_mask) { const char *name = lname->ln_name; const struct dt_key *key = (const struct dt_key *)name; @@ -92,7 +92,7 @@ __mdd_lookup(const struct lu_env *env, struct md_object *pobj, PFID(mdd_object_fid(mdd_obj))); } - rc = mdd_permission_internal_locked(env, mdd_obj, pattr, mask, + rc = mdd_permission_internal_locked(env, mdd_obj, pattr, may_mask, DT_TGT_PARENT); if (rc) RETURN(rc); diff --git a/lustre/mdd/mdd_internal.h b/lustre/mdd/mdd_internal.h index b06c021..6095259 100644 --- a/lustre/mdd/mdd_internal.h +++ b/lustre/mdd/mdd_internal.h @@ -424,9 +424,11 @@ int mdd_acl_set(const struct lu_env *env, struct mdd_object *obj, int __mdd_fix_mode_acl(const struct lu_env *env, struct lu_buf *buf, __u32 *mode); int __mdd_permission_internal(const struct lu_env *env, struct mdd_object *obj, - const struct lu_attr *la, int mask, int role); + const struct lu_attr *la, unsigned int may_mask, + int role); int mdd_permission(const struct lu_env *env, struct md_object *pobj, - struct md_object *cobj, struct md_attr *ma, int mask); + struct md_object *cobj, struct md_attr *ma, + unsigned int may_mask); int mdd_generic_thread_start(struct mdd_generic_thread *thread, int (*func)(void *), void *data, char *name); void mdd_generic_thread_stop(struct mdd_generic_thread *thread); @@ -546,18 +548,19 @@ static inline const char *mdd_obj_dev_name(const struct mdd_object *mdd_obj) static inline int mdd_permission_internal(const struct lu_env *env, struct mdd_object *obj, - const struct lu_attr *la, int mask) + const struct lu_attr *la, + unsigned int may_mask) { - return __mdd_permission_internal(env, obj, la, mask, -1); + return __mdd_permission_internal(env, obj, la, may_mask, -1); } static inline int mdd_permission_internal_locked(const struct lu_env *env, struct mdd_object *obj, const struct lu_attr *la, - int mask, + unsigned int may_mask, enum dt_object_role role) { - return __mdd_permission_internal(env, obj, la, mask, role); + return __mdd_permission_internal(env, obj, la, may_mask, role); } /* mdd inline func for calling osd_dt_object ops */ diff --git a/lustre/mdd/mdd_object.c b/lustre/mdd/mdd_object.c index eab0991..4e9a1a0 100644 --- a/lustre/mdd/mdd_object.c +++ b/lustre/mdd/mdd_object.c @@ -3077,8 +3077,8 @@ void mdd_object_make_hint(const struct lu_env *env, struct mdd_object *parent, nc->do_ops->do_ah_init(env, hint, np, nc, attr->la_mode & S_IFMT); } -static int accmode(const struct lu_env *env, const struct lu_attr *la, - u64 open_flags) +static int mdd_accmode(const struct lu_env *env, const struct lu_attr *la, + u64 open_flags) { /* Sadly, NFSD reopens a file repeatedly during operation, so the * "acc_mode = 0" allowance for newly-created files isn't honoured. @@ -3100,7 +3100,8 @@ static int mdd_open_sanity_check(const struct lu_env *env, const struct lu_attr *attr, u64 open_flags, int is_replay) { - int mode, rc; + unsigned int may_mask; + int rc; ENTRY; /* EEXIST check, also opening of *open* orphans is allowed so we can @@ -3113,13 +3114,13 @@ static int mdd_open_sanity_check(const struct lu_env *env, if (S_ISLNK(attr->la_mode)) RETURN(-ELOOP); - mode = accmode(env, attr, open_flags); + may_mask = mdd_accmode(env, attr, open_flags); - if (S_ISDIR(attr->la_mode) && (mode & MAY_WRITE)) + if (S_ISDIR(attr->la_mode) && (may_mask & MAY_WRITE)) RETURN(-EISDIR); if (!(open_flags & MDS_OPEN_CREATED)) { - rc = mdd_permission_internal(env, obj, attr, mode); + rc = mdd_permission_internal(env, obj, attr, may_mask); if (rc) RETURN(rc); } diff --git a/lustre/mdd/mdd_permission.c b/lustre/mdd/mdd_permission.c index f230845..33cb595 100644 --- a/lustre/mdd/mdd_permission.c +++ b/lustre/mdd/mdd_permission.c @@ -206,7 +206,7 @@ int __mdd_fix_mode_acl(const struct lu_env *env, struct lu_buf *buf, * Hold read_lock for obj. */ static int mdd_check_acl(const struct lu_env *env, struct mdd_object *obj, - const struct lu_attr *la, int mask) + const struct lu_attr *la, unsigned int may_mask) { #ifdef CONFIG_LUSTRE_FS_POSIX_ACL struct lu_ucred *uc = lu_ucred_assert(env); @@ -242,7 +242,7 @@ static int mdd_check_acl(const struct lu_env *env, struct mdd_object *obj, if (entry_count <= 0) RETURN(-EAGAIN); - rc = lustre_posix_acl_permission(uc, la, mask, entry, entry_count); + rc = lustre_posix_acl_permission(uc, la, may_mask, entry, entry_count); RETURN(rc); #else ENTRY; @@ -251,14 +251,15 @@ static int mdd_check_acl(const struct lu_env *env, struct mdd_object *obj, } int __mdd_permission_internal(const struct lu_env *env, struct mdd_object *obj, - const struct lu_attr *la, int mask, int role) + const struct lu_attr *la, unsigned int may_mask, + int role) { struct lu_ucred *uc = lu_ucred(env); __u32 mode; int rc; ENTRY; - if (mask == 0) + if (may_mask == 0) RETURN(0); /* These means unnecessary for permission check */ @@ -272,7 +273,7 @@ int __mdd_permission_internal(const struct lu_env *env, struct mdd_object *obj, /* * Nobody gets write access to an immutable file. */ - if ((mask & MAY_WRITE) && (la->la_valid & LA_FLAGS) && + if ((may_mask & MAY_WRITE) && (la->la_valid & LA_FLAGS) && (la->la_flags & LUSTRE_IMMUTABLE_FL)) RETURN(-EACCES); @@ -281,34 +282,34 @@ int __mdd_permission_internal(const struct lu_env *env, struct mdd_object *obj, mode = la->la_mode; if (uc->uc_fsuid == la->la_uid) { mode >>= 6; - } else { - if (mode & S_IRWXG) { - if (role != -1) - mdd_read_lock(env, obj, role); - rc = mdd_check_acl(env, obj, la, mask); - if (role != -1) - mdd_read_unlock(env, obj); - if (rc == -EACCES) - goto check_capabilities; - else if ((rc != -EAGAIN) && (rc != -EOPNOTSUPP) && - (rc != -ENODATA)) - RETURN(rc); - } - if (lustre_in_group_p(uc, la->la_gid)) - mode >>= 3; - } - - if (((mode & mask & S_IRWXO) == mask)) - RETURN(0); + } else { + if (mode & S_IRWXG) { + if (role != -1) + mdd_read_lock(env, obj, role); + rc = mdd_check_acl(env, obj, la, may_mask); + if (role != -1) + mdd_read_unlock(env, obj); + if (rc == -EACCES) + goto check_capabilities; + else if ((rc != -EAGAIN) && (rc != -EOPNOTSUPP) && + (rc != -ENODATA)) + RETURN(rc); + } + if (lustre_in_group_p(uc, la->la_gid)) + mode >>= 3; + } + + if (((mode & may_mask & S_IRWXO) == may_mask)) + RETURN(0); check_capabilities: - if (!(mask & MAY_EXEC) || + if (!(may_mask & MAY_EXEC) || (la->la_mode & S_IXUGO) || S_ISDIR(la->la_mode)) if (md_capable(uc, CAP_DAC_OVERRIDE)) RETURN(0); - if ((mask == MAY_READ) || - (S_ISDIR(la->la_mode) && !(mask & MAY_WRITE))) + if ((may_mask == MAY_READ) || + (S_ISDIR(la->la_mode) && !(may_mask & MAY_WRITE))) if (md_capable(uc, CAP_DAC_READ_SEARCH)) RETURN(0); @@ -319,7 +320,8 @@ check_capabilities: } int mdd_permission(const struct lu_env *env, struct md_object *pobj, - struct md_object *cobj, struct md_attr *ma, int mask) + struct md_object *cobj, struct md_attr *ma, + unsigned int may_mask) { struct mdd_object *mdd_pobj = NULL; struct mdd_object *mdd_cobj; @@ -344,10 +346,10 @@ int mdd_permission(const struct lu_env *env, struct md_object *pobj, RETURN(rc); rc = mdd_permission_internal_locked(env, mdd_cobj, cattr, - mask & ~MAY_RGETFACL, + may_mask & ~MAY_RGETFACL, DT_TGT_CHILD); - if (unlikely(rc == 0 && (mask & MAY_RGETFACL))) { + if (unlikely(rc == 0 && (may_mask & MAY_RGETFACL))) { if (likely(!uc)) uc = lu_ucred_assert(env); diff --git a/lustre/obdclass/acl.c b/lustre/obdclass/acl.c index c62cb20..bdf96ca 100644 --- a/lustre/obdclass/acl.c +++ b/lustre/obdclass/acl.c @@ -70,16 +70,32 @@ static inline void lustre_posix_acl_cpu_to_le(posix_acl_xattr_entry *d, * Check permission based on POSIX ACL. */ int lustre_posix_acl_permission(struct lu_ucred *mu, const struct lu_attr *la, - int want, posix_acl_xattr_entry *entry, - int count) + unsigned int may_mask, + posix_acl_xattr_entry *entry, int count) { posix_acl_xattr_entry *pa, *pe, *mask_obj; posix_acl_xattr_entry ae, me; + __u16 acl_want; int found = 0; if (count <= 0) return -EACCES; + /* There is implicit conversion between MAY_* modes and ACL_* modes. + * Don't bother explicitly converting them unless they actually change. + */ + if (0) { + acl_want = (may_mask & MAY_READ ? ACL_READ : 0) | + (may_mask & MAY_WRITE ? ACL_WRITE : 0) | + (may_mask & MAY_EXEC ? ACL_EXECUTE : 0); + } else { + BUILD_BUG_ON(MAY_READ != ACL_READ); + BUILD_BUG_ON(MAY_WRITE != ACL_WRITE); + BUILD_BUG_ON(MAY_EXEC != ACL_EXECUTE); + + acl_want = may_mask; + } + for (pa = &entry[0], pe = &entry[count - 1]; pa <= pe; pa++) { lustre_posix_acl_le_to_cpu(&ae, pa); switch (ae.e_tag) { @@ -95,14 +111,14 @@ int lustre_posix_acl_permission(struct lu_ucred *mu, const struct lu_attr *la, case ACL_GROUP_OBJ: if (lustre_in_group_p(mu, la->la_gid)) { found = 1; - if ((ae.e_perm & want) == want) + if ((ae.e_perm & acl_want) == acl_want) goto mask; } break; case ACL_GROUP: if (lustre_in_group_p(mu, ae.e_id)) { found = 1; - if ((ae.e_perm & want) == want) + if ((ae.e_perm & acl_want) == acl_want) goto mask; } break; @@ -122,7 +138,7 @@ mask: for (mask_obj = pa + 1; mask_obj <= pe; mask_obj++) { lustre_posix_acl_le_to_cpu(&me, mask_obj); if (me.e_tag == ACL_MASK) { - if ((ae.e_perm & me.e_perm & want) == want) + if ((ae.e_perm & me.e_perm & acl_want) == acl_want) return 0; return -EACCES; @@ -130,7 +146,7 @@ mask: } check_perm: - if ((ae.e_perm & want) == want) + if ((ae.e_perm & acl_want) == acl_want) return 0; return -EACCES; @@ -145,6 +161,13 @@ int lustre_posix_acl_chmod_masq(posix_acl_xattr_entry *entry, u32 mode, { posix_acl_xattr_entry *group_obj = NULL, *mask_obj = NULL, *pa, *pe; + /* There is implicit conversion between S_IRWX modes and ACL_* modes. + * Don't bother explicitly converting them unless they actually change. + */ + BUILD_BUG_ON(S_IROTH != ACL_READ); + BUILD_BUG_ON(S_IWOTH != ACL_WRITE); + BUILD_BUG_ON(S_IXOTH != ACL_EXECUTE); + for (pa = &entry[0], pe = &entry[count - 1]; pa <= pe; pa++) { switch (le16_to_cpu(pa->e_tag)) { case ACL_USER_OBJ: -- 1.8.3.1