Whamcloud - gitweb
LU-17000 utils: Coverity leaks and races 49/56149/4
authorShaun Tancheff <shaun.tancheff@hpe.com>
Tue, 27 Aug 2024 02:59:35 +0000 (09:59 +0700)
committerOleg Drokin <green@whamcloud.com>
Sun, 24 Nov 2024 06:04:46 +0000 (06:04 +0000)
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 <shaun.tancheff@hpe.com>
Change-Id: I2868d5ded322cd9cc890c463a494d296206d4be9
Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/56149
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Petros Koutoupis <petros.koutoupis@hpe.com>
Reviewed-by: Timothy Day <timday@amazon.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
lustre/utils/lfs.c
lustre/utils/liblustreapi.c

index 2e73933..0843b0f 100755 (executable)
@@ -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 {
index d27589e..d0b5eb4 100644 (file)
@@ -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);