Thread (4 messages) 4 messages, 4 authors, 2023-03-13

Re: [PATCH] Fix memory leak for function Manage_subdevs Manage_add Kill V2

From: Coly Li <hidden>
Date: 2023-02-23 15:53:47

On Wed, Feb 22, 2023 at 04:30:53PM +0800, miaoguanqin wrote:
When we test mdadm with asan,we found some memory leaks.
We fix these memory leaks based on code logic.

Signed-off-by: miaoguanqin <redacted>
---
 Assemble.c | 16 +++++++++++++---
 Kill.c     | 10 +++++++++-
 Manage.c   | 16 +++++++++++++++-
 mdadm.c    |  6 ++++++
 4 files changed, 43 insertions(+), 5 deletions(-)
[snipped]
 
quoted hunk ↗ jump to hunk
diff --git a/Kill.c b/Kill.c
index d4767e2..073288e 100644
--- a/Kill.c
+++ b/Kill.c
[snipped]
quoted hunk ↗ jump to hunk
@@ -77,6 +80,11 @@ int Kill(char *dev, struct supertype *st, int force, int
verbose, int noexcl)
 			rv = 0;
 		}
 	}
+	if (flags == 1 && st) {
+		if (st->sb)
+			free(st->sb);
May I ask why not call st->ss->free_super(st) ?

+		free(st);
+	}
 	close(fd);
 	return rv;
 }
[snipped]
quoted hunk ↗ jump to hunk
diff --git a/mdadm.c b/mdadm.c
index da66c76..981fa98 100644
--- a/mdadm.c
+++ b/mdadm.c
@@ -1765,6 +1765,12 @@ int main(int argc, char *argv[])
 		autodetect();
 		break;
 	}
+	if (ss) {
+		if (ss->sb)
+			free(ss->sb);
Same question, why not call ss->ss->free_super(ss) ?
+		free(ss);
+	
+	}
 	if (locked)
 		cluster_release_dlmlock();
 	if (mdfd > 0)

Overall the patch is fine to me. But it might be better to split it into
multiple patches that each has the changes for a single file.

Thanks.

-- 
Coly Li
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help