From f765c6ceb8a4a2415a7956498f7fdaefa477ba55 Mon Sep 17 00:00:00 2001 From: Sebastien Buisson Date: Sat, 14 Dec 2019 01:39:08 +0900 Subject: [PATCH] LU-13077 pfl: cleanup xattr checking Cleanup xattr checking in mdd and lod layers for PFL. Reported-by: Clement Barthelemy Signed-off-by: Sebastien Buisson Change-Id: I2841b615ee304785fbf316b829d8280eefc3878a Reviewed-on: https://review.whamcloud.com/37010 Reviewed-by: Olaf Faaland-LLNL Reviewed-by: Andreas Dilger Tested-by: jenkins Tested-by: Maloo Reviewed-by: Oleg Drokin --- lustre/include/uapi/linux/lustre/lustre_user.h | 6 +++++ lustre/lod/lod_object.c | 36 ++++++++++++++------------ lustre/mdd/mdd_object.c | 6 ++--- lustre/mdt/mdt_xattr.c | 9 ++----- 4 files changed, 31 insertions(+), 26 deletions(-) diff --git a/lustre/include/uapi/linux/lustre/lustre_user.h b/lustre/include/uapi/linux/lustre/lustre_user.h index bec8c97..fdcc379 100644 --- a/lustre/include/uapi/linux/lustre/lustre_user.h +++ b/lustre/include/uapi/linux/lustre/lustre_user.h @@ -755,6 +755,12 @@ static inline bool lov_pattern_supported_normal_comp(__u32 pattern) #define XATTR_LUSTRE_PREFIX "lustre." #define XATTR_LUSTRE_LOV XATTR_LUSTRE_PREFIX"lov" +/* Please update if XATTR_LUSTRE_LOV".set" groks more flags in the future */ +#define allowed_lustre_lov(att) (strcmp((att), XATTR_LUSTRE_LOV".add") == 0 || \ + strcmp((att), XATTR_LUSTRE_LOV".set") == 0 || \ + strcmp((att), XATTR_LUSTRE_LOV".set.flags") == 0 || \ + strcmp((att), XATTR_LUSTRE_LOV".del") == 0) + #define lov_user_ost_data lov_user_ost_data_v1 struct lov_user_ost_data_v1 { /* per-stripe data structure */ struct ost_id l_ost_oi; /* OST object ID */ diff --git a/lustre/lod/lod_object.c b/lustre/lod/lod_object.c index 8e3574f..0d6b06a 100644 --- a/lustre/lod/lod_object.c +++ b/lustre/lod/lod_object.c @@ -2973,6 +2973,8 @@ bool lod_last_non_stale_mirror(__u16 mirror_id, struct lod_object *lo) * the '$field' can only be 'flags' now. The xattr value is binary * lov_comp_md_v1 which contains the component ID(s) and the value of * the field to be modified. + * Please update allowed_lustre_lov macro if $field groks more values + * in the future. * * \param[in] env execution environment * \param[in] dt dt_object to be modified @@ -2998,6 +3000,9 @@ static int lod_declare_layout_set(const struct lu_env *env, bool changed = false; ENTRY; + /* Please update allowed_lustre_lov macro if op + * groks more values in the future + */ if (strcmp(op, "set.flags") != 0) { CDEBUG(D_LAYOUT, "%s: operation (%s) not supported.\n", lod2obd(d)->obd_name, op); @@ -3581,9 +3586,8 @@ static int lod_declare_xattr_set(const struct lu_env *env, strcmp(name, XATTR_LUSTRE_LOV) == 0); rc = lod_declare_layout_split(env, dt, buf, th); } else if (S_ISREG(mode) && - strlen(name) > strlen(XATTR_LUSTRE_LOV) + 1 && - strncmp(name, XATTR_LUSTRE_LOV, - strlen(XATTR_LUSTRE_LOV)) == 0) { + strlen(name) >= sizeof(XATTR_LUSTRE_LOV) + 3 && + allowed_lustre_lov(name)) { /* * this is a request to modify object's striping. * add/set/del component(s). @@ -3593,15 +3597,15 @@ static int lod_declare_xattr_set(const struct lu_env *env, rc = lod_declare_modify_layout(env, dt, name, buf, th); } else if (strncmp(name, XATTR_NAME_LMV, strlen(XATTR_NAME_LMV)) == 0 && - strlen(name) > strlen(XATTR_NAME_LMV) + 1) { - const char *op = name + strlen(XATTR_NAME_LMV) + 1; + strlen(name) > strlen(XATTR_NAME_LMV)) { + const char *op = name + strlen(XATTR_NAME_LMV); rc = -ENOTSUPP; - if (strcmp(op, "add") == 0) + if (strcmp(op, ".add") == 0) rc = lod_dir_declare_layout_add(env, dt, buf, th); - else if (strcmp(op, "del") == 0) + else if (strcmp(op, ".del") == 0) rc = lod_dir_declare_layout_delete(env, dt, buf, th); - else if (strcmp(op, "set") == 0) + else if (strcmp(op, ".set") == 0) rc = lod_sub_declare_xattr_set(env, next, buf, XATTR_NAME_LMV, fl, th); @@ -4615,23 +4619,23 @@ static int lod_xattr_set(const struct lu_env *env, RETURN(rc); } else if (S_ISDIR(dt->do_lu.lo_header->loh_attr) && strncmp(name, XATTR_NAME_LMV, strlen(XATTR_NAME_LMV)) == 0 && - strlen(name) > strlen(XATTR_NAME_LMV) + 1) { - const char *op = name + strlen(XATTR_NAME_LMV) + 1; + strlen(name) > strlen(XATTR_NAME_LMV)) { + const char *op = name + strlen(XATTR_NAME_LMV); rc = -ENOTSUPP; /* * XATTR_NAME_LMV".add" is never called, but only declared, * because lod_xattr_set_lmv() will do the addition. */ - if (strcmp(op, "del") == 0) + if (strcmp(op, ".del") == 0) rc = lod_dir_layout_delete(env, dt, buf, th); - else if (strcmp(op, "set") == 0) + else if (strcmp(op, ".set") == 0) rc = lod_sub_xattr_set(env, next, buf, XATTR_NAME_LMV, fl, th); RETURN(rc); } else if (S_ISDIR(dt->do_lu.lo_header->loh_attr) && - strcmp(name, XATTR_NAME_LOV) == 0) { + strcmp(name, XATTR_NAME_LOV) == 0) { struct lod_default_striping *lds = lod_lds_buf_get(env); struct lov_user_md_v1 *v1 = buf->lb_buf; char pool[LOV_MAXPOOLNAME + 1]; @@ -4693,9 +4697,9 @@ static int lod_xattr_set(const struct lu_env *env, th); RETURN(rc); } else if (S_ISREG(dt->do_lu.lo_header->loh_attr) && - (!strcmp(name, XATTR_NAME_LOV) || - !strncmp(name, XATTR_LUSTRE_LOV, - strlen(XATTR_LUSTRE_LOV)))) { + (strcmp(name, XATTR_NAME_LOV) == 0 || + strcmp(name, XATTR_LUSTRE_LOV) == 0 || + allowed_lustre_lov(name))) { /* in case of lov EA swap, just set it * if not, it is a replay so check striping match what we * already have during req replay, declare_xattr_set() diff --git a/lustre/mdd/mdd_object.c b/lustre/mdd/mdd_object.c index 6df8870..0613c79 100644 --- a/lustre/mdd/mdd_object.c +++ b/lustre/mdd/mdd_object.c @@ -1362,7 +1362,7 @@ static inline bool has_prefix(const char *str, const char *prefix) * * \param[in] xattr_name Full extended attribute name. * - * \return The type of changelog to use, or -1 if no changelog is to be emitted. + * \return type of changelog to use, or CL_NONE if no changelog is to be emitted */ static enum changelog_rec_type mdd_xattr_changelog_type(const struct lu_env *env, struct mdd_device *mdd, @@ -1370,8 +1370,8 @@ mdd_xattr_changelog_type(const struct lu_env *env, struct mdd_device *mdd, { /* Layout changes systematically recorded */ if (strcmp(XATTR_NAME_LOV, xattr_name) == 0 || - strncmp(XATTR_LUSTRE_LOV, xattr_name, - strlen(XATTR_LUSTRE_LOV)) == 0) + strcmp(XATTR_LUSTRE_LOV, xattr_name) == 0 || + allowed_lustre_lov(xattr_name)) return CL_LAYOUT; /* HSM information changes systematically recorded */ diff --git a/lustre/mdt/mdt_xattr.c b/lustre/mdt/mdt_xattr.c index 82d94c7..483cd45 100644 --- a/lustre/mdt/mdt_xattr.c +++ b/lustre/mdt/mdt_xattr.c @@ -534,16 +534,11 @@ int mdt_reint_setxattr(struct mdt_thread_info *info, /* ACLs were mapped out, return an error so the user knows */ if (rc != xattr_len) GOTO(out, rc = -EPERM); - } else if ((strlen(xattr_name) > strlen(XATTR_LUSTRE_LOV) + 1) && + } else if ((strlen(xattr_name) > sizeof(XATTR_LUSTRE_LOV)) && strncmp(xattr_name, XATTR_LUSTRE_LOV, strlen(XATTR_LUSTRE_LOV)) == 0) { - if (strncmp(xattr_name, XATTR_LUSTRE_LOV".add", - strlen(XATTR_LUSTRE_LOV".add")) && - strncmp(xattr_name, XATTR_LUSTRE_LOV".set", - strlen(XATTR_LUSTRE_LOV".set")) && - strncmp(xattr_name, XATTR_LUSTRE_LOV".del", - strlen(XATTR_LUSTRE_LOV".del"))) { + if (!allowed_lustre_lov(xattr_name)) { CERROR("%s: invalid xattr name: %s\n", mdt_obd_name(info->mti_mdt), xattr_name); GOTO(out, rc = -EINVAL); -- 1.8.3.1