From: Shaun Tancheff Date: Tue, 27 Aug 2024 02:59:35 +0000 (+0700) Subject: LU-17000 utils: Coverity leaks and races X-Git-Tag: 2.16.51~202 X-Git-Url: https://git.whamcloud.com/gitweb?a=commitdiff_plain;h=refs%2Fchanges%2F49%2F56149%2F4;p=fs%2Flustre-release.git LU-17000 utils: Coverity leaks and races CoverityID: 442365 ("time-of-check, time-of-use") CoverityID: 442366 ("time-of-check, time-of-use") Use fstat() on open file descriptor and use fcntl() to add the desired O_NONBLOCK flag for pipes. CoverityID: 442113 ("Resource leaks") Ensure qctl allocated object is free'd in lfs_quota() CoverityID: 442364: ("Null pointer dereferences") In lfs_fid2path() ensure `path_or_fsname` is not null before checking if it starts with a forward slash ('/') CoverityID: 442116 ("Resource leaks") In get_projid() close dir_fd as soon as it is no longer needed. CoverityID: 442373 ("Memory - corruptions, overrun allocated") Fix lov_forge_comp_v1() which also fails to clone FID Test-Parameters: trivial Test-Parameters: testlist=sanity env=ONLY="56ebb" Signed-off-by: Shaun Tancheff Change-Id: I2868d5ded322cd9cc890c463a494d296206d4be9 Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/56149 Tested-by: jenkins Tested-by: Maloo Reviewed-by: Petros Koutoupis Reviewed-by: Timothy Day Reviewed-by: Oleg Drokin --- diff --git a/lustre/utils/lfs.c b/lustre/utils/lfs.c index 2e73933..0843b0f 100755 --- a/lustre/utils/lfs.c +++ b/lustre/utils/lfs.c @@ -9882,7 +9882,7 @@ quota_type: fprintf(stderr, "%s quota: no quota type to iterate\n", progname); rc = CMD_HELP; - return rc; + goto out; } if (end_qid != 0 && start_qid > end_qid) { @@ -9890,7 +9890,7 @@ quota_type: "%s quota: end qid is smaller than start qid\n", progname); rc = CMD_HELP; - return rc; + goto out; } qctl->qc_allquota_qid_start = start_qid; @@ -10520,7 +10520,7 @@ static int lfs_fid2path(int argc, char **argv) goto out; } - if (*path_or_fsname == '/') { + if (path_or_fsname && *path_or_fsname == '/') { print_mnt_dir = true; rc = llapi_search_mounts(path_or_fsname, 0, mnt_dir, NULL); } else { diff --git a/lustre/utils/liblustreapi.c b/lustre/utils/liblustreapi.c index d27589e..d0b5eb4 100644 --- a/lustre/utils/liblustreapi.c +++ b/lustre/utils/liblustreapi.c @@ -1678,18 +1678,19 @@ again: struct stat st; did_nofollow = true; - if (stat(path, &st) != 0) - return -errno; - if (S_ISFIFO(st.st_mode)) - fd = open(path, O_RDONLY | O_NOFOLLOW | O_NONBLOCK); - else - fd = open(path, O_RDONLY | O_NOFOLLOW); + fd = open(path, O_RDONLY | O_NOFOLLOW | O_NONBLOCK); if (fd < 0) { /* restore original errno */ errno = ENOTTY; return ret; } - + if (fstat(fd, &st) != 0) { + errno = ENOTTY; + close(fd); + return ret; + } + if (!S_ISFIFO(st.st_mode)) + fcntl(fd, F_SETFL, fcntl(fd, F_GETFL) & ~O_NONBLOCK); /* close original fd and set new */ close(*d); *d = fd; @@ -3884,26 +3885,26 @@ lov_forge_comp_v1(struct lov_user_mds_data *orig, bool is_dir) struct lov_user_mds_data *new; struct lov_comp_md_v1 *comp_v1; struct lov_comp_md_entry_v1 *ent; + int lumd_hdr = offsetof(typeof(*new), lmd_lmm); int lum_off = sizeof(*comp_v1) + sizeof(*ent); int lum_size = lov_user_md_size(is_dir ? 0 : lum->lmm_stripe_count, lum->lmm_magic); - new = malloc(offsetof(typeof(*new), lmd_lmm) + lum_off + lum_size); + new = malloc(sizeof(*new) + sizeof(*ent) + lum_size); if (new == NULL) { llapi_printf(LLAPI_MSG_NORMAL, "out of memory\n"); return new; } - - memcpy(new, orig, sizeof(new->lmd_stx) + sizeof(new->lmd_flags) - + sizeof(new->lmd_lmmsize)); - + /* struct lov_user_mds_data header */ + memcpy(new, orig, lumd_hdr); + /* fill comp_v1 */ comp_v1 = (struct lov_comp_md_v1 *)&new->lmd_lmm; comp_v1->lcm_magic = lum->lmm_magic; comp_v1->lcm_size = lum_off + lum_size; comp_v1->lcm_layout_gen = is_dir ? 0 : lum->lmm_layout_gen; comp_v1->lcm_flags = 0; comp_v1->lcm_entry_count = 1; - + /* fill entry */ ent = &comp_v1->lcm_entries[0]; ent->lcme_id = 0; ent->lcme_flags = is_dir ? 0 : LCME_FL_INIT; @@ -3911,8 +3912,8 @@ lov_forge_comp_v1(struct lov_user_mds_data *orig, bool is_dir) ent->lcme_extent.e_end = LUSTRE_EOF; ent->lcme_offset = lum_off; ent->lcme_size = lum_size; - - memcpy((char *)comp_v1 + lum_off, lum, lum_size); + /* fill blob at end of entry */ + memcpy((char *)&comp_v1->lcm_entries[1], lum, lum_size); return new; } @@ -5735,6 +5736,7 @@ static int get_projid(const char *path, int *fd, mode_t mode, __u32 *projid) strncpy(lu_project.project_name, base_name, NAME_MAX); ret = ioctl(dir_fd, LL_IOC_PROJECT, &lu_project); + close(dir_fd); if (ret) { llapi_error(LLAPI_MSG_ERROR, -ENOENT, "warning: %s: failed to get xattr for '%s': %s", @@ -5742,7 +5744,6 @@ static int get_projid(const char *path, int *fd, mode_t mode, __u32 *projid) return -errno; } *projid = lu_project.project_id; - close(dir_fd); } return 0; @@ -6760,19 +6761,22 @@ static int cb_getstripe(char *path, int p, int *dp, void *data, * to get LMV just in case, and by opening it as a file but * with O_NOFOLLOW ... */ - int flag = O_RDONLY; + int flag = O_RDONLY | O_NONBLOCK; if (param->fp_no_follow) - flag = O_RDONLY | O_NOFOLLOW; - if (stat(path, &st) != 0) - return -errno; - if (S_ISFIFO(st.st_mode)) - flag |= O_NONBLOCK; + flag |= O_NOFOLLOW; fd = open(path, flag); - if (fd == -1) return 0; + if (fstat(fd, &st) != 0) { + ret = -errno; + close(fd); + return ret; + } + /* clear O_NONBLOCK for non-PIPEs */ + if (!S_ISFIFO(st.st_mode)) + fcntl(fd, F_SETFL, fcntl(fd, F_GETFL) & ~O_NONBLOCK); ret = cb_get_dirstripe(path, &fd, param); if (ret == 0) llapi_lov_dump_user_lmm(param, path, LDF_IS_DIR);