All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mateusz Kusiak <mateusz.kusiak@intel.com>
To: linux-raid@vger.kernel.org
Cc: jes@trained-monkey.org
Subject: [PATCH V2] Grow: Close cfd file descriptor
Date: Mon, 30 Aug 2021 15:32:37 +0200	[thread overview]
Message-ID: <20210830133237.7957-1-mateusz.kusiak@intel.com> (raw)

Unclosed file descriptor causes resource leak if error occurs.

Signed-off-by: Mateusz Kusiak <mateusz.kusiak@intel.com>
---
 Grow.c | 82 +++++++++++++++++++++++++---------------------------------
 1 file changed, 35 insertions(+), 47 deletions(-)

diff --git a/Grow.c b/Grow.c
index 7506ab46..dec6b742 100644
--- a/Grow.c
+++ b/Grow.c
@@ -1796,7 +1796,7 @@ int Grow_reshape(char *devname, int fd,
 	struct supertype *st;
 	char *subarray = NULL;
 
-	int frozen;
+	int frozen = 0;
 	int changed = 0;
 	char *container = NULL;
 	int cfd = -1;
@@ -1805,7 +1805,7 @@ int Grow_reshape(char *devname, int fd,
 	int added_disks;
 
 	struct mdinfo info;
-	struct mdinfo *sra;
+	struct mdinfo *sra = NULL;
 
 	if (md_get_array_info(fd, &array) < 0) {
 		pr_err("%s is not an active md array - aborting\n",
@@ -1851,14 +1851,14 @@ int Grow_reshape(char *devname, int fd,
 	}
 	if (s->raiddisks > st->max_devs) {
 		pr_err("Cannot increase raid-disks on this array beyond %d\n", st->max_devs);
-		return 1;
+		goto error;
 	}
 	if (s->level == 0 && (array.state & (1 << MD_SB_BITMAP_PRESENT)) &&
 		!(array.state & (1 << MD_SB_CLUSTERED)) && !st->ss->external) {
 		array.state &= ~(1 << MD_SB_BITMAP_PRESENT);
 		if (md_set_array_info(fd, &array) != 0) {
 			pr_err("failed to remove internal bitmap.\n");
-			return 1;
+			goto error;
 		}
 	}
 
@@ -1880,16 +1880,14 @@ int Grow_reshape(char *devname, int fd,
 		}
 		if (cfd < 0) {
 			pr_err("Unable to open container for %s\n", devname);
-			free(subarray);
-			return 1;
+			goto error;
 		}
 
 		retval = st->ss->load_container(st, cfd, NULL);
 
 		if (retval) {
 			pr_err("Cannot read superblock for %s\n", devname);
-			free(subarray);
-			return 1;
+			goto error;
 		}
 
 		/* check if operation is supported for metadata handler */
@@ -1900,6 +1898,7 @@ int Grow_reshape(char *devname, int fd,
 			cc = st->ss->container_content(st, subarray);
 			for (content = cc; content ; content = content->next) {
 				int allow_reshape = 1;
+				rv = 1;
 
 				/* check if reshape is allowed based on metadata
 				 * indications stored in content.array.status
@@ -1913,26 +1912,23 @@ int Grow_reshape(char *devname, int fd,
 				if (!allow_reshape) {
 					pr_err("cannot reshape arrays in container with unsupported metadata: %s(%s)\n",
 					       devname, container);
-					sysfs_free(cc);
-					free(subarray);
-					return 1;
+					break;
 				}
 				if (content->consistency_policy ==
 				    CONSISTENCY_POLICY_PPL) {
 					pr_err("Operation not supported when ppl consistency policy is enabled\n");
-					sysfs_free(cc);
-					free(subarray);
-					return 1;
+					break;
 				}
 				if (content->consistency_policy ==
 				    CONSISTENCY_POLICY_BITMAP) {
 					pr_err("Operation not supported when write-intent bitmap is enabled\n");
-					sysfs_free(cc);
-					free(subarray);
-					return 1;
+					break;
 				}
+				rv = 0;
 			}
 			sysfs_free(cc);
+			if (rv == 1)
+				goto release;
 		}
 		if (mdmon_running(container))
 			st->update_tail = &st->updates;
@@ -1950,7 +1946,7 @@ int Grow_reshape(char *devname, int fd,
 		       s->raiddisks - array.raid_disks,
 		       s->raiddisks - array.raid_disks == 1 ? "" : "s",
 		       array.spare_disks + added_disks);
-		return 1;
+		goto error;
 	}
 
 	sra = sysfs_read(fd, NULL, GET_LEVEL | GET_DISKS | GET_DEVS |
@@ -1963,17 +1959,15 @@ int Grow_reshape(char *devname, int fd,
 	} else {
 		pr_err("failed to read sysfs parameters for %s\n",
 			devname);
-		return 1;
+		goto error;
 	}
 	frozen = freeze(st);
 	if (frozen < -1) {
 		/* freeze() already spewed the reason */
-		sysfs_free(sra);
-		return 1;
+		goto error;
 	} else if (frozen < 0) {
 		pr_err("%s is performing resync/recovery and cannot be reshaped\n", devname);
-		sysfs_free(sra);
-		return 1;
+		goto error;
 	}
 
 	/* ========= set size =============== */
@@ -1989,15 +1983,13 @@ int Grow_reshape(char *devname, int fd,
 
 		if (orig_size == 0) {
 			pr_err("Cannot set device size in this type of array.\n");
-			rv = 1;
-			goto release;
+			goto error;
 		}
 
 		if (reshape_super(st, s->size, UnSet, UnSet, 0, 0, UnSet, NULL,
 				  devname, APPLY_METADATA_CHANGES,
 				  c->verbose > 0)) {
-			rv = 1;
-			goto release;
+			goto error;
 		}
 		sync_metadata(st);
 		if (st->ss->external) {
@@ -2126,8 +2118,7 @@ size_change_error:
 			if (err == EBUSY &&
 			    (array.state & (1<<MD_SB_BITMAP_PRESENT)))
 				cont_err("Bitmap must be removed before size can be changed\n");
-			rv = 1;
-			goto release;
+			goto error;
 		}
 		if (s->assume_clean) {
 			/* This will fail on kernels older than 3.0 unless
@@ -2183,10 +2174,7 @@ size_change_error:
 		err = remove_disks_for_takeover(st, sra, array.layout);
 		if (err) {
 			dprintf("Array cannot be reshaped\n");
-			if (cfd > -1)
-				close(cfd);
-			rv = 1;
-			goto release;
+			goto error;
 		}
 		/* Make sure mdmon has seen the device removal
 		 * and updated metadata before we continue with
@@ -2200,8 +2188,7 @@ size_change_error:
 	info.array = array;
 	if (sysfs_init(&info, fd, NULL)) {
 		pr_err("failed to initialize sysfs.\n");
-		rv = 1;
-		goto release;
+		goto error;
 	}
 	strcpy(info.text_version, sra->text_version);
 	info.component_size = s->size*2;
@@ -2222,8 +2209,7 @@ size_change_error:
 			pr_err("%s has a non-standard layout.  If you wish to preserve this\n", devname);
 			cont_err("during the reshape, please specify --layout=preserve\n");
 			cont_err("If you want to change it, specify a layout or use --layout=normalise\n");
-			rv = 1;
-			goto release;
+			goto error;
 		}
 	} else if (strcmp(s->layout_str, "normalise") == 0 ||
 		   strcmp(s->layout_str, "normalize") == 0) {
@@ -2239,8 +2225,7 @@ size_change_error:
 			}
 		} else {
 			pr_err("%s is only meaningful when reshaping a RAID6 array.\n", s->layout_str);
-			rv = 1;
-			goto release;
+			goto error;
 		}
 	} else if (strcmp(s->layout_str, "preserve") == 0) {
 		/* This means that a non-standard RAID6 layout
@@ -2261,8 +2246,7 @@ size_change_error:
 			info.new_layout = map_name(r6layout, l);
 		} else {
 			pr_err("%s in only meaningful when reshaping to RAID6\n", s->layout_str);
-			rv = 1;
-			goto release;
+			goto error;
 		}
 	} else {
 		int l = info.new_level;
@@ -2283,14 +2267,12 @@ size_change_error:
 			break;
 		default:
 			pr_err("layout not meaningful with this level\n");
-			rv = 1;
-			goto release;
+			goto error;
 		}
 		if (info.new_layout == UnSet) {
 			pr_err("layout %s not understood for this level\n",
 				s->layout_str);
-			rv = 1;
-			goto release;
+			goto error;
 		}
 	}
 
@@ -2359,8 +2341,7 @@ size_change_error:
 				  info.array.raid_disks, info.delta_disks,
 				  c->backup_file, devname,
 				  APPLY_METADATA_CHANGES, c->verbose)) {
-			rv = 1;
-			goto release;
+			goto error;
 		}
 		sync_metadata(st);
 		rv = reshape_array(container, fd, devname, st, &info, c->force,
@@ -2369,10 +2350,17 @@ size_change_error:
 		frozen = 0;
 	}
 release:
+	if (cfd > -1)
+		close(cfd);
+	free(subarray);
 	sysfs_free(sra);
 	if (frozen > 0)
 		unfreeze(st);
+	free(st);
 	return rv;
+error:
+	rv = 1;
+	goto release;
 }
 
 /* verify_reshape_position()
-- 
2.26.2


             reply	other threads:[~2021-08-30 13:32 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-30 13:32 Mateusz Kusiak [this message]
2021-10-08 10:52 ` [PATCH V2] Grow: Close cfd file descriptor Jes Sorensen
2021-10-14 14:18   ` Kusiak, Mateusz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210830133237.7957-1-mateusz.kusiak@intel.com \
    --to=mateusz.kusiak@intel.com \
    --cc=jes@trained-monkey.org \
    --cc=linux-raid@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.