[PATCH v2 0/6] rebased patch set from Wu Guanghao

[PATCH v2 0/6] rebased patch set from Wu Guanghao

[Coly Li]

This is the rebased patch set from Wu Guanghao, all the patches can
be applied on top of commit 0a07dea8d3b78 ("Mdmonitor: Refactor
check_one_sharer() for better error handling") from the mdadm tree.

The patch from me is to solve conflict for the first patch of Guanghao.
In order to make the commit logs to be more understandable, I recompose
some patch subjects or commit logs.

The V2 series only changes the patch subject of mine by the suggestion
from Paul Menzel. And nothing more changed from V1.

Thank you in advance for taking them.

Coly Li
---
Coly Li (1):
  util.c: reorder code lines in parse_layout_faulty()

Wu Guanghao (5):
  util.c: fix memleak in parse_layout_faulty()
  Detail.c: fix memleak in Detail()
  isuper-intel.c: fix double free in load_imsm_mpb()
  super-intel.c: fix memleak in find_disk_attached_hba()
  super-ddf.c: fix memleak in get_vd_num_of_subarray()

 Detail.c      |  1 +
 super-ddf.c   |  9 +++++++--
 super-intel.c |  5 +++--
 util.c        | 11 ++++++++---
 4 files changed, 19 insertions(+), 7 deletions(-)

-- 
2.39.2

[PATCH v2 1/6] util.c: reorder code lines in parse_layout_faulty()

[Coly Li]

Resort the code lines in parse_layout_faulty() to make it more
comfortable, no logic change.

Signed-off-by: Coly Li <colyli@suse.de>
Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
---
 util.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/util.c b/util.c
index 7fc881bf..b0b7aec4 100644
--- a/util.c
+++ b/util.c
@@ -421,12 +421,15 @@ int parse_layout_10(char *layout)
 
 int parse_layout_faulty(char *layout)
 {
+	int ln, mode;
+	char *m;
+
 	if (!layout)
 		return -1;
+
 	/* Parse the layout string for 'faulty' */
-	int ln = strcspn(layout, "0123456789");
-	char *m = xstrdup(layout);
-	int mode;
+	ln = strcspn(layout, "0123456789");
+	m = xstrdup(layout);
 	m[ln] = 0;
 	mode = map_name(faultylayout, m);
 	if (mode == UnSet)
-- 
2.39.2

[PATCH v2 2/6] util.c: fix memleak in parse_layout_faulty()

[Coly Li]

From: Wu Guanghao <wuguanghao3@huawei.com>

char *m is allocated by xstrdup but not free() before return, will cause
a memory leak

Signed-off-by: Wu Guanghao <wuguanghao3@huawei.com>
Acked-by: Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com>
Acked-by: Coly Li <colyli@suse.de>
---
 util.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/util.c b/util.c
index b0b7aec4..9f1e1f7c 100644
--- a/util.c
+++ b/util.c
@@ -432,6 +432,8 @@ int parse_layout_faulty(char *layout)
 	m = xstrdup(layout);
 	m[ln] = 0;
 	mode = map_name(faultylayout, m);
+	free(m);
+
 	if (mode == UnSet)
 		return -1;
 
-- 
2.39.2

[PATCH v2 3/6] Detail.c: fix memleak in Detail()

[Coly Li]

From: Wu Guanghao <wuguanghao3@huawei.com>

char *sysdev = xstrdup() but not free() in for loop, will cause memory
leak

Signed-off-by: Wu Guanghao <wuguanghao3@huawei.com>
Acked-by: Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com>
Acked-by: Coly Li <colyli@suse.de>
---
 Detail.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Detail.c b/Detail.c
index ce7a8445..4ef26460 100644
--- a/Detail.c
+++ b/Detail.c
@@ -303,6 +303,7 @@ int Detail(char *dev, struct context *c)
 				if (path)
 					printf("MD_DEVICE_%s_DEV=%s\n",
 					       sysdev, path);
+				free(sysdev);
 			}
 		}
 		goto out;
-- 
2.39.2

[PATCH v2 4/6] isuper-intel.c: fix double free in load_imsm_mpb()

[Coly Li]

From: Wu Guanghao <wuguanghao3@huawei.com>

In load_imsm_mpb() there is potential double free issue on super->buf.

The first location to free super->buf is from get_super_block() <==
load_and_parse_mpb() <== load_imsm_mpb():
 4514         if (posix_memalign(&super->migr_rec_buf, MAX_SECTOR_SIZE,
 4515             MIGR_REC_BUF_SECTORS*MAX_SECTOR_SIZE) != 0) {
 4516                 pr_err("could not allocate migr_rec buffer\n");
 4517                 free(super->buf);
 4518                 return 2;
 4519         }

If the above error condition happens, super->buf is freed and value 2
is returned to get_super_block() eventually. Then in the following code
block inside load_imsm_mpb(),
 5289  error:
 5290         if (!err) {
 5291                 s->next = *super_list;
 5292                 *super_list = s;
 5293         } else {
 5294                 if (s)
 5295                         free_imsm(s);
 5296                 close_fd(&dfd);
 5297         }
at line 5295 when free_imsm() is called, super->buf is freed again from
the call chain free_imsm() <== __free_imsm(), in following code block,
 4651         if (super->buf) {
 4652                 free(super->buf);
 4653                 super->buf = NULL;
 4654         }

This patch sets super->buf as NULL after line 4517 in load_imsm_mpb()
to avoid the potential double free().

(Coly Li helps to re-compose the commit log)

Signed-off-by: Wu Guanghao <wuguanghao3@huawei.com>
Reviewed-by: Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com>
Acked-by: Coly Li <colyli@suse.de>
---
 super-intel.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/super-intel.c b/super-intel.c
index 89fac626..4a3da847 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -4515,6 +4515,7 @@ static int load_imsm_mpb(int fd, struct intel_super *super, char *devname)
 	    MIGR_REC_BUF_SECTORS*MAX_SECTOR_SIZE) != 0) {
 		pr_err("could not allocate migr_rec buffer\n");
 		free(super->buf);
+		super->buf = NULL;
 		return 2;
 	}
 	super->clean_migration_record_by_mdmon = 0;
-- 
2.39.2

[PATCH v2 5/6] super-intel.c: fix memleak in find_disk_attached_hba()

[Coly Li]

From: Wu Guanghao <wuguanghao3@huawei.com>

If disk_path = diskfd_to_devpath(), we need free(disk_path) before
return, otherwise there will be a memory leak

Signed-off-by: Wu Guanghao <wuguanghao3@huawei.com>
Reviewed-by: Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com>
Acked-by: Coly Li <colyli@suse.de>
---
 super-intel.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/super-intel.c b/super-intel.c
index 4a3da847..e155a8ae 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -713,12 +713,12 @@ static struct sys_dev* find_disk_attached_hba(int fd, const char *devname)
 
 	for (elem = list; elem; elem = elem->next)
 		if (path_attached_to_hba(disk_path, elem->path))
-			return elem;
+			break;
 
 	if (disk_path != devname)
 		free(disk_path);
 
-	return NULL;
+	return elem;
 }
 
 static int find_intel_hba_capability(int fd, struct intel_super *super,
-- 
2.39.2

[PATCH v2 6/6] super-ddf.c: fix memleak in get_vd_num_of_subarray()

[Coly Li]

From: Wu Guanghao <wuguanghao3@huawei.com>

sra = sysfs_read() should be free before return in
get_vd_num_of_subarray()

Signed-off-by: Wu Guanghao <wuguanghao3@huawei.com>
Acked-by: Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com>
Acked-by: Coly Li <colyli@suse.de>
---
 super-ddf.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index 309812df..b86c6acd 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -1592,15 +1592,20 @@ static unsigned int get_vd_num_of_subarray(struct supertype *st)
 	sra = sysfs_read(-1, st->devnm, GET_VERSION);
 	if (!sra || sra->array.major_version != -1 ||
 	    sra->array.minor_version != -2 ||
-	    !is_subarray(sra->text_version))
+	    !is_subarray(sra->text_version)) {
+		if (sra)
+			sysfs_free(sra);
 		return DDF_NOTFOUND;
+	}
 
 	sub = strchr(sra->text_version + 1, '/');
 	if (sub != NULL)
 		vcnum = strtoul(sub + 1, &end, 10);
 	if (sub == NULL || *sub == '\0' || *end != '\0' ||
-	    vcnum >= be16_to_cpu(ddf->active->max_vd_entries))
+	    vcnum >= be16_to_cpu(ddf->active->max_vd_entries)) {
+		sysfs_free(sra);
 		return DDF_NOTFOUND;
+	}
 
 	return vcnum;
 }
-- 
2.39.2

Re: [PATCH v2 0/6] rebased patch set from Wu Guanghao

[Jes Sorensen]

On 3/3/23 11:21, Coly Li wrote:
> This is the rebased patch set from Wu Guanghao, all the patches can
> be applied on top of commit 0a07dea8d3b78 ("Mdmonitor: Refactor
> check_one_sharer() for better error handling") from the mdadm tree.
> 
> The patch from me is to solve conflict for the first patch of Guanghao.
> In order to make the commit logs to be more understandable, I recompose
> some patch subjects or commit logs.
> 
> The V2 series only changes the patch subject of mine by the suggestion
> from Paul Menzel. And nothing more changed from V1.
> 
> Thank you in advance for taking them.
> 
> Coly Li

All applied!

Thanks Coly rebasing these!

Jes