LU-17816 llapi: ensure pool name is nul terminated

strncpy() usage is inconsistent about the size of pool name
and sometimes for get to ensure a nul byte is placed at the
end of the copy.

CoverityID: 397181 ("Buffer not null terminated (BUFFER_SIZE)")

Also cleanup a case of checking that an unsigned value >= 0

CoverityID: 397820 ("Unsigned compared against 0 (NO_EFFECT)")

Lustre-change: https://review.whamcloud.com/55018
Lustre-commit: 64469274a4f3e202c76cf9a2757b8f36e8d0ee08

Signed-off-by: Shaun Tancheff <shaun.tancheff@hpe.com>
Change-Id: Idec7adaf89c9dabc0275687c4a069fc8fa63e7a7
Reviewed-on: https://review.whamcloud.com/c/ex/lustre-release/+/55119
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>

diff --git a/lustre/include/uapi/linux/lustre/lustre_user.h b/lustre/include/uapi/linux/lustre/lustre_user.h
index 20cbfcd..17e1906 100644 (file)
--- a/lustre/include/uapi/linux/lustre/lustre_user.h
+++ b/lustre/include/uapi/linux/lustre/lustre_user.h
@@ -1207,7 +1207,7 @@ struct lmv_user_md_v1 {
        __u32   lum_padding2;
        __u32   lum_padding3;
        char    lum_pool_name[LOV_MAXPOOLNAME + 1];
-       struct  lmv_user_mds_data  lum_objects[0];
+       struct  lmv_user_mds_data  lum_objects[];
 } __attribute__((packed));
 
 static inline __u32 lmv_foreign_to_md_stripes(__u32 size)

diff --git a/lustre/utils/lfs.c b/lustre/utils/lfs.c
index 62f06ca..0f75f73 100644 (file)
--- a/lustre/utils/lfs.c
+++ b/lustre/utils/lfs.c
@@ -4527,11 +4527,9 @@ static int lfs_setstripe_internal(int argc, char **argv,
                if (overstriped)
                        lmu->lum_hash_type |= LMV_HASH_FLAG_OVERSTRIPED;
 
-               if (lsa.lsa_pool_name) {
-                       strncpy(lmu->lum_pool_name, lsa.lsa_pool_name,
-                               sizeof(lmu->lum_pool_name) - 1);
-                       lmu->lum_pool_name[sizeof(lmu->lum_pool_name) - 1] = 0;
-               }
+               if (lsa.lsa_pool_name)
+                       snprintf(lmu->lum_pool_name, sizeof(lmu->lum_pool_name),
+                                "%s", lsa.lsa_pool_name);
                if (lsa.lsa_nr_tgts > 1) {
                        int i;
 
@@ -5749,8 +5747,8 @@ err_free:
                         * We do check for empty pool because empty pool
                         * is used to find V1 LOV attributes
                         */
-                       strncpy(param.fp_poolname, optarg, LOV_MAXPOOLNAME);
-                       param.fp_poolname[LOV_MAXPOOLNAME] = '\0';
+                       snprintf(param.fp_poolname, sizeof(param.fp_poolname),
+                                "%s", optarg);
                        param.fp_exclude_pool = !!neg_opt;
                        param.fp_check_pool = 1;
                        break;
@@ -7904,7 +7902,8 @@ quota_type:
                case LFS_POOL_OPT:
                        if (lfs_verify_poolarg(optarg))
                                return -1;
-                       strncpy(qctl->qc_poolname, optarg, LOV_MAXPOOLNAME);
+                       snprintf(qctl->qc_poolname, LOV_MAXPOOLNAME + 1, "%s",
+                                optarg);
                        qctl->qc_cmd  = LUSTRE_Q_SETINFOPOOL;
                        break;
                /* getopt prints error message for us when opterr != 0 */
@@ -8263,7 +8262,8 @@ quota_type_def:
                                rc = -1;
                                goto out;
                        }
-                       strncpy(qctl->qc_poolname, optarg, LOV_MAXPOOLNAME);
+                       snprintf(qctl->qc_poolname, LOV_MAXPOOLNAME + 1, "%s",
+                                optarg);
                        qctl->qc_cmd = qctl->qc_cmd == LUSTRE_Q_SETDEFAULT ?
                                                LUSTRE_Q_SETDEFAULT_POOL :
                                                LUSTRE_Q_SETQUOTAPOOL;
@@ -9098,7 +9098,8 @@ quota_type:
                                rc = -1;
                                goto out;
                        }
-                       strncpy(qctl->qc_poolname, optarg, LOV_MAXPOOLNAME);
+                       snprintf(qctl->qc_poolname, LOV_MAXPOOLNAME + 1, "%s",
+                                optarg);
                        qctl->qc_cmd = qctl->qc_cmd == LUSTRE_Q_GETINFO ?
                                                LUSTRE_Q_GETINFOPOOL :
                                                LUSTRE_Q_GETQUOTAPOOL;

diff --git a/lustre/utils/liblustreapi.c b/lustre/utils/liblustreapi.c
index 8718c13..c77534e 100644 (file)
--- a/lustre/utils/liblustreapi.c
+++ b/lustre/utils/liblustreapi.c
@@ -673,7 +673,8 @@ retry_open:
                struct lov_user_md_v3 *lumv3 = (void *)lum;
 
                lumv3->lmm_magic = LOV_USER_MAGIC_V3;
-               strncpy(lumv3->lmm_pool_name, pool_name, LOV_MAXPOOLNAME);
+               snprintf(lumv3->lmm_pool_name, sizeof(lumv3->lmm_pool_name),
+                        "%s", pool_name);
        }
        if (param->lsp_is_specific) {
                struct lov_user_md_v3 *lumv3 = (void *)lum;
@@ -686,7 +687,8 @@ retry_open:
                         * OST list, therefore if pool is not specified we have
                         * to pack a null pool name for placeholder.
                         */
-                       memset(lumv3->lmm_pool_name, 0, LOV_MAXPOOLNAME);
+                       memset(lumv3->lmm_pool_name, 0,
+                              sizeof(lumv3->lmm_pool_name));
                }
 
                for (i = 0; i < param->lsp_stripe_count; i++)
@@ -957,14 +959,15 @@ static inline void param2lmu(struct lmv_user_md *lmu,
        lmu->lum_hash_type = param->lsp_stripe_pattern;
        lmu->lum_max_inherit = param->lsp_max_inherit;
        lmu->lum_max_inherit_rr = param->lsp_max_inherit_rr;
-       if (param->lsp_pool != NULL)
-               strncpy(lmu->lum_pool_name, param->lsp_pool, LOV_MAXPOOLNAME);
        if (param->lsp_is_specific) {
                int i;
 
                for (i = 0; i < param->lsp_stripe_count; i++)
                        lmu->lum_objects[i].lum_mds = param->lsp_tgts[i];
        }
+       if (param->lsp_pool)
+               snprintf(lmu->lum_pool_name, sizeof(lmu->lum_pool_name), "%s",
+                        param->lsp_pool);
 }
 
 int llapi_dir_set_default_lmv(const char *name,

diff --git a/lustre/utils/liblustreapi_layout.c b/lustre/utils/liblustreapi_layout.c
index 0264600..a27aa7b 100644 (file)
--- a/lustre/utils/liblustreapi_layout.c
+++ b/lustre/utils/liblustreapi_layout.c
@@ -916,9 +916,9 @@ llapi_layout_to_lum(const struct llapi_layout *layout)
                                (struct lov_user_md_v3 *)blob;
 
                        if (comp->llc_pool_name[0] != '\0') {
-                               strncpy(lumv3->lmm_pool_name,
-                                       comp->llc_pool_name,
-                                       sizeof(lumv3->lmm_pool_name));
+                               snprintf(lumv3->lmm_pool_name,
+                                        sizeof(lumv3->lmm_pool_name), "%s",
+                                        comp->llc_pool_name);
                        } else {
                                memset(lumv3->lmm_pool_name, 0,
                                       sizeof(lumv3->lmm_pool_name));
@@ -1918,7 +1918,7 @@ int llapi_layout_pool_name_get(const struct llapi_layout *layout, char *dest,
                return -1;
        }
 
-       strncpy(dest, comp->llc_pool_name, n);
+       snprintf(dest, n, "%s", comp->llc_pool_name);
 
        return 0;
 }
@@ -1951,7 +1951,8 @@ int llapi_layout_pool_name_set(struct llapi_layout *layout,
                return -1;
        }
 
-       strncpy(comp->llc_pool_name, pool_name, sizeof(comp->llc_pool_name));
+       snprintf(comp->llc_pool_name, sizeof(comp->llc_pool_name), "%s",
+                pool_name);
        return 0;
 }
 
@@ -2133,7 +2134,7 @@ __u16 llapi_layout_string_flags(char *string)
  */
 static bool llapi_layout_mirror_count_is_valid(uint16_t count)
 {
-       return count >= 0 && count <= LUSTRE_MIRROR_COUNT_MAX;
+       return count <= LUSTRE_MIRROR_COUNT_MAX;
 }
 
 /**
@@ -2443,7 +2444,7 @@ int llapi_layout_comp_add(struct llapi_layout *layout)
         * component, the end of the previous component cannot be EOF.) */
        if (llapi_layout_comp_extent_set(layout, last->llc_extent.e_end,
                                        LUSTRE_EOF - 1)) {
-               llapi_layout_comp_del(layout);
+               (void)llapi_layout_comp_del(layout);
                layout->llot_is_composite = composite;
                return -1;
        }
@@ -3123,8 +3124,8 @@ int llapi_layout_merge(struct llapi_layout **dst_layout,
                new->llc_stripe_offset = comp->llc_stripe_offset;
 
                if (comp->llc_pool_name[0] != '\0')
-                       strncpy(new->llc_pool_name, comp->llc_pool_name,
-                               sizeof(new->llc_pool_name));
+                       snprintf(new->llc_pool_name, sizeof(new->llc_pool_name),
+                                "%s", comp->llc_pool_name);
 
                for (i = 0; i < comp->llc_objects_count; i++) {
                        if (__llapi_comp_objects_realloc(new,
@@ -3155,6 +3156,7 @@ int llapi_layout_merge(struct llapi_layout **dst_layout,
        return 0;
 error:
        llapi_layout_free(new_layout);
+       *dst_layout = NULL;
        return -1;
 }
 

diff --git a/lustre/utils/obd.c b/lustre/utils/obd.c
index 103a3b0..9a4c2ad 100644 (file)
--- a/lustre/utils/obd.c
+++ b/lustre/utils/obd.c
@@ -5389,8 +5389,8 @@ int llog_poollist(char *fsname, char *poolname)
        lpld.lpld_exists = false;
        strncpy(lpld.lpld_fsname, fsname, sizeof(lpld.lpld_fsname) - 1);
        if (poolname && poolname[0])
-               strncpy(lpld.lpld_poolname, poolname,
-                       sizeof(lpld.lpld_poolname) - 1);
+               snprintf(lpld.lpld_poolname, sizeof(lpld.lpld_poolname), "%s",
+                        poolname);
        snprintf(logname, sizeof(logname), "%s-client", fsname);
        rc = jt_llog_print_iter(logname, 0, -1, llog_poollist_cb, &lpld, false,
                                false);