LU-5757 hsm: strengthen checks for flags and archive id

Prior to this patch undefined flags bits and out of range
archive id can be set.
Also changed the concerned error handling that has been
recently added (LU-5732) as part of sanity-hsm/test_500.

Signed-off-by: Bruno Faccini <bruno.faccini@intel.com>
Change-Id: I64403de4529f0214bab55c2fc13281b0a3d30a11
Reviewed-on: http://review.whamcloud.com/13337
Tested-by: Jenkins
Tested-by: Maloo <hpdd-maloo@intel.com>
Reviewed-by: frank zago <fzago@cray.com>
Reviewed-by: Henri Doreau <henri.doreau@cea.fr>
Reviewed-by: Oleg Drokin <oleg.drokin@intel.com>

diff --git a/lustre/include/lustre/lustre_idl.h b/lustre/include/lustre/lustre_idl.h
index cd2bb40..2ccb7ce 100644 (file)
--- a/lustre/include/lustre/lustre_idl.h
+++ b/lustre/include/lustre/lustre_idl.h
@@ -369,6 +369,13 @@ extern void lustre_som_swab(struct som_attrs *attrs);
 
 #define SOM_INCOMPAT_SUPP 0x0
 
+/* copytool uses a 32b bitmask field to encode archive-Ids during register
+ * with MDT thru kuc.
+ * archive num = 0 => all
+ * archive num from 1 to 32
+ */
+#define LL_HSM_MAX_ARCHIVE (sizeof(__u32) * 8)
+
 /**
  * HSM on-disk attributes stored in a separate xattr.
  */

diff --git a/lustre/llite/file.c b/lustre/llite/file.c
index 7d9c922..99b790f 100644 (file)
--- a/lustre/llite/file.c
+++ b/lustre/llite/file.c
@@ -2106,6 +2106,11 @@ static int ll_hsm_state_set(struct inode *inode, struct hsm_state_set *hss)
 {
        struct md_op_data       *op_data;
        int                      rc;
+       ENTRY;
+
+       /* Detect out-of range masks */
+       if ((hss->hss_setmask | hss->hss_clearmask) & ~HSM_FLAGS_MASK)
+               RETURN(-EINVAL);
 
        /* Non-root users are forbidden to set or clear flags which are
         * NOT defined in HSM_USER_MASK. */
@@ -2113,6 +2118,11 @@ static int ll_hsm_state_set(struct inode *inode, struct hsm_state_set *hss)
            !cfs_capable(CFS_CAP_SYS_ADMIN))
                RETURN(-EPERM);
 
+       /* Detect out-of range archive id */
+       if ((hss->hss_valid & HSS_ARCHIVE_ID) &&
+           (hss->hss_archive_id > LL_HSM_MAX_ARCHIVE))
+               RETURN(-EINVAL);
+
        op_data = ll_prep_md_op_data(NULL, inode, NULL, NULL, 0, 0,
                                     LUSTRE_OPC_ANY, hss);
        if (IS_ERR(op_data))

diff --git a/lustre/mdt/mdt_hsm.c b/lustre/mdt/mdt_hsm.c
index 237663a..34d9fda 100644 (file)
--- a/lustre/mdt/mdt_hsm.c
+++ b/lustre/mdt/mdt_hsm.c
@@ -289,11 +289,23 @@ int mdt_hsm_state_set(struct tgt_session_info *tsi)
                mdt_set_capainfo(info, 0, &info->mti_body->mbo_fid1,
                            req_capsule_client_get(info->mti_pill, &RMF_CAPA1));
 
+       /* Detect out-of range masks */
+       if ((hss->hss_setmask | hss->hss_clearmask) & ~HSM_FLAGS_MASK) {
+               CDEBUG(D_HSM, "Incompatible masks provided (set "LPX64
+                      ", clear "LPX64") vs supported set (%#x).\n",
+                      hss->hss_setmask, hss->hss_clearmask, HSM_FLAGS_MASK);
+               GOTO(out_unlock, rc = -EINVAL);
+       }
+
        /* Non-root users are forbidden to set or clear flags which are
         * NOT defined in HSM_USER_MASK. */
        if (((hss->hss_setmask | hss->hss_clearmask) & ~HSM_USER_MASK) &&
-           !md_capable(mdt_ucred(info), CFS_CAP_SYS_ADMIN))
+           !md_capable(mdt_ucred(info), CFS_CAP_SYS_ADMIN)) {
+               CDEBUG(D_HSM, "Incompatible masks provided (set "LPX64
+                      ", clear "LPX64") vs unprivileged set (%#x).\n",
+                      hss->hss_setmask, hss->hss_clearmask, HSM_USER_MASK);
                GOTO(out_unlock, rc = -EPERM);
+       }
 
        /* Read current HSM info */
        ma->ma_valid = 0;
@@ -316,6 +328,14 @@ int mdt_hsm_state_set(struct tgt_session_info *tsi)
                               PFID(&info->mti_body->mbo_fid1));
                        GOTO(out_unlock, rc);
                }
+
+               /* Detect out-of range archive id */
+               if (hss->hss_archive_id > LL_HSM_MAX_ARCHIVE) {
+                       CDEBUG(D_HSM, "archive id %u exceeds maximum %zu.\n",
+                              hss->hss_archive_id, LL_HSM_MAX_ARCHIVE);
+                       GOTO(out_unlock, rc = -EINVAL);
+               }
+
                ma->ma_hsm.mh_arch_id = hss->hss_archive_id;
        }
 

diff --git a/lustre/tests/llapi_hsm_test.c b/lustre/tests/llapi_hsm_test.c
index 397dc8c..54c3495 100644 (file)
--- a/lustre/tests/llapi_hsm_test.c
+++ b/lustre/tests/llapi_hsm_test.c
@@ -373,53 +373,94 @@ void test51(void)
                        hus.hus_archive_id, i);
        }
 
-       /* Bugs following. This should not succeed. Builds the following file:
-        *
-        *   $ ../utils/lfs hsm_state /mnt/lustre/hsm_check_test
-        *
-        *   /mnt/lustre/hsm_check_test: (0x8008007d) released exists
-        *     archived never_release never_archive lost_from_hsm,
-        *     archive_id:-1789
-        */
-
        /* Invalid archive numbers */
        rc = llapi_hsm_state_set_fd(fd, HS_EXISTS, 0, 33);
-       ASSERTF(rc == 0, "llapi_hsm_state_set_fd failed: %s", strerror(-rc));
+       ASSERTF(rc == -EINVAL, "llapi_hsm_state_set_fd: %s", strerror(-rc));
 
        rc = llapi_hsm_state_set_fd(fd, HS_EXISTS, 0, 151);
-       ASSERTF(rc == 0, "llapi_hsm_state_set_fd failed: %s", strerror(-rc));
+       ASSERTF(rc == -EINVAL, "llapi_hsm_state_set_fd: %s", strerror(-rc));
 
        rc = llapi_hsm_state_set_fd(fd, HS_EXISTS, 0, -1789);
-       ASSERTF(rc == 0, "llapi_hsm_state_set_fd failed: %s", strerror(-rc));
+       ASSERTF(rc == -EINVAL, "llapi_hsm_state_set_fd: %s", strerror(-rc));
 
-       /* Setable + Unsettable flags */
+       /* Settable flags, with respect of the HSM file state transition rules:
+        *      DIRTY without EXISTS: no dirty if no archive was created
+        *      DIRTY and RELEASED: a dirty file could not be released
+        *      RELEASED without ARCHIVED: do not release a non-archived file
+        *      LOST without ARCHIVED: cannot lost a non-archived file.
+        */
        rc = llapi_hsm_state_set_fd(fd, HS_DIRTY, 0, 0);
        ASSERTF(rc == 0, "llapi_hsm_state_set_fd failed: %s", strerror(-rc));
 
+       rc = llapi_hsm_state_set_fd(fd, 0, HS_EXISTS, 0);
+       ASSERTF(rc == -EINVAL, "llapi_hsm_state_set_fd failed: %s",
+               strerror(-rc));
+
        rc = llapi_hsm_state_set_fd(fd, 0, HS_DIRTY, 0);
        ASSERTF(rc == 0, "llapi_hsm_state_set_fd failed: %s", strerror(-rc));
 
+       rc = llapi_hsm_state_set_fd(fd, 0, HS_EXISTS, 0);
+       ASSERTF(rc == 0, "llapi_hsm_state_set_fd failed: %s", strerror(-rc));
+
+       rc = llapi_hsm_state_set_fd(fd, HS_DIRTY, 0, 0);
+       ASSERTF(rc == -EINVAL, "llapi_hsm_state_set_fd failed: %s",
+               strerror(-rc));
+
+       rc = llapi_hsm_state_set_fd(fd, HS_RELEASED, 0, 0);
+       ASSERTF(rc == -EINVAL, "llapi_hsm_state_set_fd failed: %s",
+               strerror(-rc));
+
+       rc = llapi_hsm_state_set_fd(fd, HS_LOST, 0, 0);
+       ASSERTF(rc == -EINVAL, "llapi_hsm_state_set_fd failed: %s",
+               strerror(-rc));
+
        rc = llapi_hsm_state_set_fd(fd, HS_ARCHIVED, 0, 0);
        ASSERTF(rc == 0, "llapi_hsm_state_set_fd failed: %s", strerror(-rc));
 
        rc = llapi_hsm_state_set_fd(fd, HS_RELEASED, 0, 0);
        ASSERTF(rc == 0, "llapi_hsm_state_set_fd failed: %s", strerror(-rc));
 
+       rc = llapi_hsm_state_set_fd(fd, HS_LOST, 0, 0);
+       ASSERTF(rc == 0, "llapi_hsm_state_set_fd failed: %s", strerror(-rc));
+
+       rc = llapi_hsm_state_set_fd(fd, HS_DIRTY|HS_EXISTS, 0, 0);
+       ASSERTF(rc == -EINVAL, "llapi_hsm_state_set_fd failed: %s",
+               strerror(-rc));
+
+       rc = llapi_hsm_state_set_fd(fd, 0, HS_RELEASED, 0);
+       ASSERTF(rc == 0, "llapi_hsm_state_set_fd failed: %s", strerror(-rc));
+
+       rc = llapi_hsm_state_set_fd(fd, HS_DIRTY|HS_EXISTS, 0, 0);
+       ASSERTF(rc == 0, "llapi_hsm_state_set_fd failed: %s", strerror(-rc));
+
+       rc = llapi_hsm_state_set_fd(fd, 0, HS_ARCHIVED, 0);
+       ASSERTF(rc == -EINVAL, "llapi_hsm_state_set_fd failed: %s",
+               strerror(-rc));
+
+       rc = llapi_hsm_state_set_fd(fd, 0, HS_LOST, 0);
+       ASSERTF(rc == 0, "llapi_hsm_state_set_fd failed: %s", strerror(-rc));
+
+       rc = llapi_hsm_state_set_fd(fd, 0, HS_ARCHIVED, 0);
+       ASSERTF(rc == 0, "llapi_hsm_state_set_fd failed: %s", strerror(-rc));
+
        rc = llapi_hsm_state_set_fd(fd, HS_NORELEASE, 0, 0);
        ASSERTF(rc == 0, "llapi_hsm_state_set_fd failed: %s", strerror(-rc));
 
+       rc = llapi_hsm_state_set_fd(fd, 0, HS_NORELEASE, 0);
+       ASSERTF(rc == 0, "llapi_hsm_state_set_fd failed: %s", strerror(-rc));
+
        rc = llapi_hsm_state_set_fd(fd, HS_NOARCHIVE, 0, 0);
        ASSERTF(rc == 0, "llapi_hsm_state_set_fd failed: %s", strerror(-rc));
 
-       rc = llapi_hsm_state_set_fd(fd, HS_LOST, 0, 0);
+       rc = llapi_hsm_state_set_fd(fd, 0, HS_NOARCHIVE, 0);
        ASSERTF(rc == 0, "llapi_hsm_state_set_fd failed: %s", strerror(-rc));
 
        /* Bogus flags for good measure. */
        rc = llapi_hsm_state_set_fd(fd, 0x00080000, 0, 0);
-       ASSERTF(rc == 0, "llapi_hsm_state_set_fd failed: %s", strerror(-rc));
+       ASSERTF(rc == -EINVAL, "llapi_hsm_state_set_fd: %s", strerror(-rc));
 
        rc = llapi_hsm_state_set_fd(fd, 0x80000000, 0, 0);
-       ASSERTF(rc == 0, "llapi_hsm_state_set_fd failed: %s", strerror(-rc));
+       ASSERTF(rc == -EINVAL, "llapi_hsm_state_set_fd: %s", strerror(-rc));
 
        close(fd);
 }

diff --git a/lustre/utils/lhsmtool_posix.c b/lustre/utils/lhsmtool_posix.c
index 12ead32..0442d55 100644 (file)
--- a/lustre/utils/lhsmtool_posix.c
+++ b/lustre/utils/lhsmtool_posix.c
@@ -65,12 +65,6 @@
 # define NSEC_PER_SEC 1000000000UL
 #endif
 
-/* copytool uses a 32b bitmask field to register with kuc
- * archive num = 0 => all
- * archive num from 1 to 32
- */
-#define MAX_ARCHIVE_CNT (sizeof(__u32) * 8)
-
 enum ct_action {
        CA_IMPORT = 1,
        CA_REBIND,
@@ -86,7 +80,7 @@ struct options {
        int                      o_verbose;
        int                      o_copy_xattrs;
        int                      o_archive_cnt;
-       int                      o_archive_id[MAX_ARCHIVE_CNT];
+       int                      o_archive_id[LL_HSM_MAX_ARCHIVE];
        int                      o_report_int;
        unsigned long long       o_bandwidth;
        size_t                   o_chunk_size;
@@ -246,11 +240,11 @@ static int ct_parseopts(int argc, char * const *argv)
                                long_opts, NULL)) != -1) {
                switch (c) {
                case 'A':
-                       if ((opt.o_archive_cnt >= MAX_ARCHIVE_CNT) ||
-                           (atoi(optarg) >= MAX_ARCHIVE_CNT)) {
+                       if ((opt.o_archive_cnt >= LL_HSM_MAX_ARCHIVE) ||
+                           (atoi(optarg) >= LL_HSM_MAX_ARCHIVE)) {
                                rc = -E2BIG;
                                CT_ERROR(rc, "archive number must be less"
-                                        "than %zu", MAX_ARCHIVE_CNT);
+                                        "than %zu", LL_HSM_MAX_ARCHIVE);
                                return rc;
                        }
                        opt.o_archive_id[opt.o_archive_cnt] = atoi(optarg);

diff --git a/lustre/utils/liblustreapi_hsm.c b/lustre/utils/liblustreapi_hsm.c
index ebc9183..b4dac67 100644 (file)
--- a/lustre/utils/liblustreapi_hsm.c
+++ b/lustre/utils/liblustreapi_hsm.c
@@ -680,6 +680,13 @@ int llapi_hsm_copytool_register(struct hsm_copytool_private **priv,
                return -EINVAL;
        }
 
+       if (archive_count > LL_HSM_MAX_ARCHIVE) {
+               llapi_err_noerrno(LLAPI_MSG_ERROR, "%d requested when maximum "
+                                 "of %zu archives supported", archive_count,
+                                 LL_HSM_MAX_ARCHIVE);
+               return -EINVAL;
+       }
+
        ct = calloc(1, sizeof(*ct));
        if (ct == NULL)
                return -ENOMEM;
@@ -717,10 +724,10 @@ int llapi_hsm_copytool_register(struct hsm_copytool_private **priv,
        /* no archives specified means "match all". */
        ct->archives = 0;
        for (rc = 0; rc < archive_count; rc++) {
-               if (archives[rc] > 8 * sizeof(ct->archives)) {
-                       llapi_err_noerrno(LLAPI_MSG_ERROR,
-                                         "maximum of %zu archives supported",
-                                         8 * sizeof(ct->archives));
+               if ((archives[rc] > LL_HSM_MAX_ARCHIVE) || (archives[rc] < 0)) {
+                       llapi_err_noerrno(LLAPI_MSG_ERROR, "%d requested when "
+                                         "archive id [0 - %zu] is supported",
+                                         archives[rc], LL_HSM_MAX_ARCHIVE);
                        rc = -EINVAL;
                        goto out_err;
                }