From: Shaun Tancheff Date: Mon, 6 May 2024 09:26:22 +0000 (+0700) Subject: LU-17816 llapi: ensure pool name is nul terminated X-Git-Tag: 2.15.64~170 X-Git-Url: https://git.whamcloud.com/?a=commitdiff_plain;h=64469274a4f3e202c76cf9a2757b8f36e8d0ee08;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)") Signed-off-by: Shaun Tancheff Change-Id: Idec7adaf89c9dabc0275687c4a069fc8fa63e7a7 Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/55018 Tested-by: jenkins Tested-by: Maloo Reviewed-by: Andreas Dilger Reviewed-by: Petros Koutoupis Reviewed-by: Oleg Drokin --- diff --git a/lustre/include/uapi/linux/lustre/lustre_user.h b/lustre/include/uapi/linux/lustre/lustre_user.h index abf779a..2901297 100644 --- 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 --- 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 --- 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 --- 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 --- 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);