From 3c36158566061a515a2730b9a64a607b73eae02e Mon Sep 17 00:00:00 2001 From: Arshad Hussain Date: Fri, 13 Oct 2023 13:57:38 +0530 Subject: [PATCH] LU-17000 coverity: Fix Resource Leak(2) This patch fixes error reported by coverity run. CoverityID: 397140 ("Resource leak"): lfs.c CoverityID: 397370 ("Resource leak"): dir.c CoverityID: 397378 ("Resource leak"): obd.c CoverityID: 397406 ("Resource leak"): obd.c Signed-off-by: Arshad Hussain Change-Id: I92bbf6749d667631b92868bcaf78af558c250441 Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/52686 Tested-by: jenkins Tested-by: Andreas Dilger Tested-by: Maloo Reviewed-by: Patrick Farrell Reviewed-by: Andreas Dilger Reviewed-by: Oleg Drokin --- lustre/llite/dir.c | 22 +++++++++++++--------- lustre/utils/lfs.c | 41 ++++++++++++++++++++++++----------------- lustre/utils/obd.c | 21 ++++++++++++++++----- 3 files changed, 53 insertions(+), 31 deletions(-) diff --git a/lustre/llite/dir.c b/lustre/llite/dir.c index 2856bb7..253fbf7 100644 --- a/lustre/llite/dir.c +++ b/lustre/llite/dir.c @@ -1387,10 +1387,11 @@ static int ll_rmfid(struct file *file, void __user *arg) const struct fid_array __user *ufa = arg; struct inode *inode = file_inode(file); struct ll_sb_info *sbi = ll_i2sbi(inode); - struct fid_array *lfa = NULL; - size_t size; - unsigned int nr; + struct fid_array *lfa = NULL, *lfa_new = NULL; int i, rc, *rcs = NULL; + unsigned int nr; + bool lfa_flag = false; /* lfa already free'ed */ + size_t size; ENTRY; if (!capable(CAP_DAC_READ_SEARCH) && @@ -1418,11 +1419,12 @@ static int ll_rmfid(struct file *file, void __user *arg) * for which we want to remove FID are visible in the namespace. */ if (!fid_is_root(&sbi->ll_root_fid)) { - struct fid_array *lfa_new = NULL; int path_len = PATH_MAX, linkno; struct getinfo_fid2path *gf; int idx, last_idx = nr - 1; + lfa_new = NULL; + OBD_ALLOC(lfa_new, size); if (!lfa_new) GOTO(free_rcs, rc = -ENOMEM); @@ -1430,7 +1432,7 @@ static int ll_rmfid(struct file *file, void __user *arg) gf = kmalloc(sizeof(*gf) + path_len + 1, GFP_NOFS); if (!gf) - GOTO(free_rcs, rc = -ENOMEM); + GOTO(free_lfa_new, rc = -ENOMEM); for (idx = 0; idx < nr; idx++) { linkno = 0; @@ -1451,8 +1453,7 @@ static int ll_rmfid(struct file *file, void __user *arg) GFP_NOFS); if (!tmpgf) { kfree(gf); - OBD_FREE(lfa_new, size); - GOTO(free_rcs, rc = -ENOMEM); + GOTO(free_lfa_new, rc = -ENOMEM); } gf = tmpgf; continue; @@ -1483,9 +1484,9 @@ static int ll_rmfid(struct file *file, void __user *arg) } kfree(gf); OBD_FREE(lfa, size); + lfa_flag = true; lfa = lfa_new; } - if (lfa->fa_nr == 0) GOTO(free_rcs, rc = rcs[nr - 1]); @@ -1500,10 +1501,13 @@ static int ll_rmfid(struct file *file, void __user *arg) rc = -EFAULT; } +free_lfa_new: + OBD_FREE(lfa_new, size); free_rcs: OBD_FREE_PTR_ARRAY(rcs, nr); free_lfa: - OBD_FREE(lfa, size); + if (!lfa_flag) + OBD_FREE(lfa, size); RETURN(rc); } diff --git a/lustre/utils/lfs.c b/lustre/utils/lfs.c index 6db3263..87828b4 100644 --- a/lustre/utils/lfs.c +++ b/lustre/utils/lfs.c @@ -10570,8 +10570,10 @@ static int lfs_hsm_request(int argc, char **argv, int action) char *mntpath = NULL; int rc; - if (argc < 2) - return CMD_HELP; + if (argc < 2) { + rc = CMD_HELP; + goto out_cmd_help; + } while ((c = getopt_long(argc, argv, "a:D:hl:m:", long_opts, NULL)) != -1) { @@ -10587,7 +10589,8 @@ static int lfs_hsm_request(int argc, char **argv, int action) action != HUA_REMOVE) { fprintf(stderr, "error: -a is supported only when archiving or removing\n"); - return CMD_HELP; + rc = CMD_HELP; + goto out_cmd_help; } archive_id = atoi(optarg); break; @@ -10602,15 +10605,18 @@ static int lfs_hsm_request(int argc, char **argv, int action) progname, argv[optind - 1]); fallthrough; case 'h': - return CMD_HELP; + rc = CMD_HELP; + goto out_cmd_help; } } /* All remaining args are files, so we have at least nbfile */ nbfile = argc - optind; - if ((nbfile == 0) && (!filelist)) - return CMD_HELP; + if ((nbfile == 0) && (!filelist)) { + rc = errno; + goto out_errno; + } if (opaque) opaque_len = strlen(opaque); @@ -10623,7 +10629,8 @@ static int lfs_hsm_request(int argc, char **argv, int action) if (!hur) { fprintf(stderr, "Cannot create the request: %s\n", strerror(errno)); - return errno; + rc = errno; + goto out_errno; } nbfile_alloc = nbfile; @@ -10639,7 +10646,7 @@ static int lfs_hsm_request(int argc, char **argv, int action) rc = fill_hur_item(hur, i, mntpath, argv[optind + i], &last_dev); if (rc) - goto out_free; + goto out_hur; } /* from here stop using nb_file, use hur->hur_request.hr_itemcount */ @@ -10651,7 +10658,7 @@ static int lfs_hsm_request(int argc, char **argv, int action) fprintf(stderr, "Cannot read the file list %s: %s\n", filelist, strerror(errno)); rc = -errno; - goto out_free; + goto out_hur; } while ((rc = getline(&line, &len, fp)) != -1) { @@ -10673,7 +10680,7 @@ static int lfs_hsm_request(int argc, char **argv, int action) hur = oldhur; rc = -errno; fclose(fp); - goto out_free; + goto out_hur; } size = hur_len(oldhur); if (size < 0) { @@ -10685,7 +10692,7 @@ static int lfs_hsm_request(int argc, char **argv, int action) hur = oldhur; rc = -E2BIG; fclose(fp); - goto out_free; + goto out_hur; } memcpy(hur, oldhur, size); free(oldhur); @@ -10699,7 +10706,7 @@ static int lfs_hsm_request(int argc, char **argv, int action) mntpath, line, &last_dev); if (rc) { fclose(fp); - goto out_free; + goto out_hur; } if (!some_file) { @@ -10723,15 +10730,15 @@ static int lfs_hsm_request(int argc, char **argv, int action) some_file, strerror(errno)); } rc = llapi_hsm_request(fullpath, hur); - if (rc) { + if (rc) fprintf(stderr, "Cannot send HSM request (use of %s): %s\n", some_file, strerror(-rc)); - goto out_free; - } -out_free: - free(some_file); +out_hur: free(hur); +out_errno: + free(some_file); +out_cmd_help: return rc; } diff --git a/lustre/utils/obd.c b/lustre/utils/obd.c index 313418d..a4e6219 100644 --- a/lustre/utils/obd.c +++ b/lustre/utils/obd.c @@ -3287,26 +3287,35 @@ static char *get_event_filter(__u32 cmd) * \retval 1 if ostname is found * 0 if ostname is not found * -ENOENT if ostname is deleted + * -ENOMEM if Error allocating memory */ static int llog_search_ost_cb(const char *record, void *data) { char *ostname = data; char ost_filter[MAX_STRING_SIZE] = {'\0'}; char *add_osc, *del_osc, *setup, *cleanup; - int rc = 0; + int rc = -ENOMEM; add_osc = get_event_filter(LCFG_LOV_ADD_OBD); + if (!add_osc) + goto out; + del_osc = get_event_filter(LCFG_LOV_DEL_OBD); + if (!del_osc) + goto out; + setup = get_event_filter(LCFG_SETUP); + if (!setup) + goto out; + cleanup = get_event_filter(LCFG_CLEANUP); - if (!add_osc || !del_osc || !setup || !cleanup) { - rc = -ENOMEM; + if (!cleanup) goto out; - } if (ostname && ostname[0]) snprintf(ost_filter, sizeof(ost_filter), " %s,", ostname); + rc = 0; if (strstr(record, ost_filter)) { if (strstr(record, add_osc) || strstr(record, setup)) { rc = 1; @@ -5824,8 +5833,10 @@ int jt_changelog_deregister(int argc, char **argv) optind++; } - if (optind < argc || argc == 1) + if (optind < argc || argc == 1) { + free(username); return CMD_HELP; + } data.ioc_dev = cur_device; data.ioc_u32_1 = id; -- 1.8.3.1