LU-13077 pfl: cleanup xattr checking

Cleanup xattr checking in mdd and lod layers for PFL.

Reported-by: Clement Barthelemy <clement.barthelemy@nextino.eu>
Signed-off-by: Sebastien Buisson <sbuisson@ddn.com>
Change-Id: I2841b615ee304785fbf316b829d8280eefc3878a
Reviewed-on: https://review.whamcloud.com/37010
Reviewed-by: Olaf Faaland-LLNL <faaland1@llnl.gov>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

diff --git a/lustre/include/uapi/linux/lustre/lustre_user.h b/lustre/include/uapi/linux/lustre/lustre_user.h
index bec8c97..fdcc379 100644 (file)
--- 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"
 
 #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 */
 #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 (file)
--- 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.
  * 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
  *
  * \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;
 
        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);
        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) &&
                        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).
                /*
                 * 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 &&
 
                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;
 
                rc = -ENOTSUPP;

-               if (strcmp(op, "add") == 0)
+               if (strcmp(op, ".add") == 0)

                        rc = lod_dir_declare_layout_add(env, dt, buf, th);
                        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);
                        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);
 
                        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 &&
                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.
                 */
 
                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);
                        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) &&
                        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];
                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) &&
                                                      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()
                /* 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 (file)
--- 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.
  *
  *
  * \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,
  */
 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 ||
 {
        /* 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 */
                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 (file)
--- 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);
                /* 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) {
 
                   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);
                        CERROR("%s: invalid xattr name: %s\n",
                               mdt_obd_name(info->mti_mdt), xattr_name);
                        GOTO(out, rc = -EINVAL);