From: Shaun Tancheff Date: Wed, 15 May 2024 06:30:39 +0000 (-0700) Subject: LU-17816 llapi: ensure pool name is nul terminated X-Git-Url: https://git.whamcloud.com/gitweb?a=commitdiff_plain;h=9520a4e4e522c181535100dd5530fb2a17c2b0ee;p=fs%2Flustre-release.git 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 Change-Id: Idec7adaf89c9dabc0275687c4a069fc8fa63e7a7 Reviewed-on: https://review.whamcloud.com/c/ex/lustre-release/+/55119 Reviewed-by: Andreas Dilger Tested-by: jenkins Tested-by: Maloo --- diff --git a/lustre/include/uapi/linux/lustre/lustre_user.h b/lustre/include/uapi/linux/lustre/lustre_user.h index 20cbfcd..17e1906 100644 --- 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 --- 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 --- 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 --- 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 --- 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);