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)")

Signed-off-by: Shaun Tancheff <shaun.tancheff@hpe.com>
Change-Id: Idec7adaf89c9dabc0275687c4a069fc8fa63e7a7
Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/55018
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Petros Koutoupis <petros.koutoupis@hpe.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 abf779a..2901297 100644 (file)
--- a/lustre/include/uapi/linux/lustre/lustre_user.h
+++ b/lustre/include/uapi/linux/lustre/lustre_user.h
@@ -1239,7 +1239,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 a5f23c1..d8487b7 100644 (file)
--- a/lustre/utils/lfs.c
+++ b/lustre/utils/lfs.c
@@ -4401,11 +4401,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;
 
@@ -6049,8 +6047,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;
@@ -8202,7 +8200,8 @@ static int lfs_setquota_times(int argc, char **argv, struct if_quotactl *qctl)
                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;
                case 't': /* Yes, of course! */
@@ -8581,7 +8580,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;
@@ -8675,8 +8675,8 @@ quota_type_def:
 
                if (qctl->qc_cmd == LUSTRE_Q_SETQUOTAPOOL) {
                        tmp_qctl->qc_cmd = LUSTRE_Q_GETQUOTAPOOL;
-                       strncpy(tmp_qctl->qc_poolname, qctl->qc_poolname,
-                               LOV_MAXPOOLNAME);
+                       snprintf(tmp_qctl->qc_poolname, LOV_MAXPOOLNAME + 1,
+                                "%s", qctl->qc_poolname);
                } else {
                        tmp_qctl->qc_cmd  = LUSTRE_Q_GETQUOTA;
                }
@@ -9588,8 +9588,8 @@ static int lfs_quota(int argc, char **argv)
                                        rc = -EINVAL;
                                        goto out;
                                }
-                               strncpy(qctl->qc_poolname, optarg,
-                                       LOV_MAXPOOLNAME);
+                               snprintf(qctl->qc_poolname,
+                                        LOV_MAXPOOLNAME + 1, "%s", optarg);
                                if (qctl->qc_cmd == LUSTRE_Q_GETINFO)
                                        qctl->qc_cmd = LUSTRE_Q_GETINFOPOOL;
                                else
@@ -9771,7 +9771,8 @@ quota_type:
                        }
                        p++;
                        printf("Quotas for pool: %s\n", p);
-                       strncpy(qctl->qc_poolname, p, LOV_MAXPOOLNAME);
+                       snprintf(qctl->qc_poolname, LOV_MAXPOOLNAME + 1, "%s",
+                               p);
                        rc = get_print_quota(mnt, name, qctl, verbose, quiet,
                                             human_readable, show_default);
                        if (rc)

diff --git a/lustre/utils/liblustreapi.c b/lustre/utils/liblustreapi.c
index 464f711..2fedf9b 100644 (file)
--- a/lustre/utils/liblustreapi.c
+++ b/lustre/utils/liblustreapi.c
@@ -618,7 +618,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;
@@ -631,7 +632,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++)
@@ -902,14 +904,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 5db3d94..310d283 100644 (file)
--- a/lustre/utils/liblustreapi_layout.c
+++ b/lustre/utils/liblustreapi_layout.c
@@ -893,9 +893,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));
@@ -1723,7 +1723,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;
 }
@@ -1756,7 +1756,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;
 }
 
@@ -1943,7 +1944,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;
 }
 
 /**
@@ -2222,7 +2223,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;
        }
@@ -2933,8 +2934,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,
@@ -2962,6 +2963,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 c871ea9..81e7cd1 100644 (file)
--- a/lustre/utils/obd.c
+++ b/lustre/utils/obd.c
@@ -4971,8 +4971,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);
@@ -5102,7 +5102,7 @@ int parse_pool_cmd_args(int argc, char **argv, bool *wait,
                return -ENAMETOOLONG;
        }
 
-       strncpy(poolname, ptr, LOV_MAXPOOLNAME + 1);
+       snprintf(poolname, LOV_MAXPOOLNAME + 1, "%s", ptr);
        if (lov_pool_is_reserved(poolname)) {
                fprintf(stderr, "%s: poolname cannot be '%s'\n",
                        cmdname, poolname);