Whamcloud - gitweb
LU-13040 lmv: Pool name string handling 08/36908/13
authorShaun Tancheff <shaun.tancheff@hpe.com>
Thu, 20 Feb 2020 17:11:14 +0000 (11:11 -0600)
committerOleg Drokin <green@whamcloud.com>
Sun, 1 Mar 2020 05:35:45 +0000 (05:35 +0000)
KASAN picked up a case where pool_name may not be null
terminated.

There are a few cases where strncpy is used, however strncpy does
not guarantee the target string is null terminated. The preference
is to use strlcpy.

Cray-bug-id: LUS-8229
Signed-off-by: Shaun Tancheff <shaun.tancheff@hpe.com>
Change-Id: I414e955b5964c24b061edd76ed9e64a8985c537d
Reviewed-on: https://review.whamcloud.com/36908
Tested-by: jenkins <devops@whamcloud.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Ben Evans <beevans@whamcloud.com>
Reviewed-by: Petros Koutoupis <petros.koutoupis@hpe.com>
Tested-by: Maloo <maloo@whamcloud.com>
lustre/include/lustre_lmv.h
lustre/lmv/lmv_obd.c
lustre/mgs/mgs_handler.c

index d32b4ac..5e03321 100644 (file)
@@ -98,8 +98,8 @@ lsm_md_eq(const struct lmv_stripe_md *lsm1, const struct lmv_stripe_md *lsm2)
                                lsm2->lsm_md_migrate_offset ||
            lsm1->lsm_md_migrate_hash !=
                                lsm2->lsm_md_migrate_hash ||
-           strcmp(lsm1->lsm_md_pool_name,
-                     lsm2->lsm_md_pool_name) != 0)
+           strncmp(lsm1->lsm_md_pool_name, lsm2->lsm_md_pool_name,
+                   sizeof(lsm1->lsm_md_pool_name)) != 0)
                return false;
 
        if (lmv_dir_striped(lsm1)) {
@@ -117,12 +117,16 @@ static inline void lsm_md_dump(int mask, const struct lmv_stripe_md *lsm)
 {
        int i;
 
-       CDEBUG(mask, "magic %#x stripe count %d master mdt %d hash type %#x "
-               "version %d migrate offset %d migrate hash %#x pool %s\n",
-               lsm->lsm_md_magic, lsm->lsm_md_stripe_count,
-               lsm->lsm_md_master_mdt_index, lsm->lsm_md_hash_type,
-               lsm->lsm_md_layout_version, lsm->lsm_md_migrate_offset,
-               lsm->lsm_md_migrate_hash, lsm->lsm_md_pool_name);
+       /* If lsm_md_magic == LMV_MAGIC_FOREIGN pool_name may not be a null
+        * terminated string so only print LOV_MAXPOOLNAME bytes.
+        */
+       CDEBUG(mask,
+              "magic %#x stripe count %d master mdt %d hash type %#x version %d migrate offset %d migrate hash %#x pool %.*s\n",
+              lsm->lsm_md_magic, lsm->lsm_md_stripe_count,
+              lsm->lsm_md_master_mdt_index, lsm->lsm_md_hash_type,
+              lsm->lsm_md_layout_version, lsm->lsm_md_migrate_offset,
+              lsm->lsm_md_migrate_hash,
+              LOV_MAXPOOLNAME, lsm->lsm_md_pool_name);
 
        if (!lmv_dir_striped(lsm))
                return;
index 5ab0ebf..be17eb0 100644 (file)
@@ -3144,6 +3144,7 @@ static inline int lmv_unpack_user_md(struct obd_export *exp,
        lsm->lsm_md_stripe_count = le32_to_cpu(lmu->lum_stripe_count);
        lsm->lsm_md_master_mdt_index = le32_to_cpu(lmu->lum_stripe_offset);
        lsm->lsm_md_hash_type = le32_to_cpu(lmu->lum_hash_type);
+       lsm->lsm_md_pool_name[LOV_MAXPOOLNAME] = 0;
 
        return 0;
 }
index e5603d5..90c34fd 100644 (file)
@@ -740,7 +740,7 @@ static int mgs_extract_fs_pool(char *arg, char *fsname, char *poolname)
        /* Also make sure poolname is not to long. */
        if (strlen(ptr) > LOV_MAXPOOLNAME)
                return -ENAMETOOLONG;
-       strncpy(poolname, ptr, LOV_MAXPOOLNAME);
+       strlcpy(poolname, ptr, LOV_MAXPOOLNAME + 1);
 
        /* Test if fsname is empty */
        len = strlen(arg) - strlen(ptr) - 1;