* [PATCH v2 1/6] e2fsck: set s->len=0 if malloc() fails in, alloc_string()
2021-12-31 7:40 [PATCH v2 0/6] solve memory leak and check whether NULL pointer zhanchengbin
@ 2021-12-31 7:41 ` zhanchengbin
2021-12-31 7:42 ` [PATCH v2 2/6] lib/ss: check whether argp is null before accessing it, in ss_execute_command() zhanchengbin
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: zhanchengbin @ 2021-12-31 7:41 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: linux-ext4, liuzhiqiang26, linfeilong, wubo40
In alloc_string(), when malloc fails, len in the
string structure should be 0.
Signed-off-by: zhanchengbin <zhanchengbin1@huawei.com>
---
e2fsck/logfile.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/e2fsck/logfile.c b/e2fsck/logfile.c
index 63e9a12f..7bdeae19 100644
--- a/e2fsck/logfile.c
+++ b/e2fsck/logfile.c
@@ -32,7 +32,7 @@ static void alloc_string(struct string *s, int len)
{
s->s = malloc(len);
/* e2fsck_allocate_memory(ctx, len, "logfile name"); */
- s->len = len;
+ s->len = s->s ? len : 0;
s->end = 0;
}
--
2.27.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 2/6] lib/ss: check whether argp is null before accessing it, in ss_execute_command()
2021-12-31 7:40 [PATCH v2 0/6] solve memory leak and check whether NULL pointer zhanchengbin
2021-12-31 7:41 ` [PATCH v2 1/6] e2fsck: set s->len=0 if malloc() fails in, alloc_string() zhanchengbin
@ 2021-12-31 7:42 ` zhanchengbin
2021-12-31 7:42 ` [PATCH v2 3/6] lib/support: check whether inump is null before, accessing it in quota_set_sb_inum() zhanchengbin
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: zhanchengbin @ 2021-12-31 7:42 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: linux-ext4, liuzhiqiang26, linfeilong, wubo40
In ss_execute_command(), we should check whether argp is null before
accessing it,
otherwise the null potinter dereference error may occur.
Signed-off-by: zhanchengbin <zhanchengbin1@huawei.com>
---
lib/ss/execute_cmd.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lib/ss/execute_cmd.c b/lib/ss/execute_cmd.c
index d443a468..b6e4a5dc 100644
--- a/lib/ss/execute_cmd.c
+++ b/lib/ss/execute_cmd.c
@@ -171,6 +171,8 @@ int ss_execute_command(int sci_idx, register char
*argv[])
for (argp = argv; *argp; argp++)
argc++;
argp = (char **)malloc((argc+1)*sizeof(char *));
+ if (argp == NULL)
+ return (SS_ET_COMMAND_NOT_FOUND);
for (i = 0; i <= argc; i++)
argp[i] = argv[i];
i = really_execute_command(sci_idx, argc, &argp);
--
2.27.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 3/6] lib/support: check whether inump is null before, accessing it in quota_set_sb_inum()
2021-12-31 7:40 [PATCH v2 0/6] solve memory leak and check whether NULL pointer zhanchengbin
2021-12-31 7:41 ` [PATCH v2 1/6] e2fsck: set s->len=0 if malloc() fails in, alloc_string() zhanchengbin
2021-12-31 7:42 ` [PATCH v2 2/6] lib/ss: check whether argp is null before accessing it, in ss_execute_command() zhanchengbin
@ 2021-12-31 7:42 ` zhanchengbin
2021-12-31 7:42 ` [PATCH v2 4/6] e2fsprogs: call ext2fs_badblocks_list_free() to free, list in exception branch zhanchengbin
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: zhanchengbin @ 2021-12-31 7:42 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: linux-ext4, liuzhiqiang26, linfeilong, wubo40
In quota_set_sb_inum(), we should check whether inump is null before
accessing it,
otherwise the null potinter dereference error may occur.
Signed-off-by: zhanchengbin <zhanchengbin1@huawei.com>
---
lib/support/mkquota.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/support/mkquota.c b/lib/support/mkquota.c
index 6f4a0b90..482e3d91 100644
--- a/lib/support/mkquota.c
+++ b/lib/support/mkquota.c
@@ -99,7 +99,8 @@ void quota_set_sb_inum(ext2_filsys fs, ext2_ino_t ino,
enum quota_type qtype)
log_debug("setting quota ino in superblock: ino=%u, type=%d", ino,
qtype);
- *inump = ino;
+ if (inump != NULL)
+ *inump = ino;
ext2fs_mark_super_dirty(fs);
}
--
2.27.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 4/6] e2fsprogs: call ext2fs_badblocks_list_free() to free, list in exception branch
2021-12-31 7:40 [PATCH v2 0/6] solve memory leak and check whether NULL pointer zhanchengbin
` (2 preceding siblings ...)
2021-12-31 7:42 ` [PATCH v2 3/6] lib/support: check whether inump is null before, accessing it in quota_set_sb_inum() zhanchengbin
@ 2021-12-31 7:42 ` zhanchengbin
2021-12-31 7:43 ` [PATCH v2 5/6] e2fsck: check whether ldesc is null before accessing, it in end_problem_latch() zhanchengbin
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: zhanchengbin @ 2021-12-31 7:42 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: linux-ext4, liuzhiqiang26, linfeilong, wubo40
In the exception branch,if we donot call ext2fs_badblocks_list_free() to
free bb_list|badblock_list, memory leak will occur.
Signed-off-by: zhanchengbin <zhanchengbin1@huawei.com>
---
misc/dumpe2fs.c | 1 +
resize/resize2fs.c | 4 ++--
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/misc/dumpe2fs.c b/misc/dumpe2fs.c
index 3f4fc4ed..ef6d1cb8 100644
--- a/misc/dumpe2fs.c
+++ b/misc/dumpe2fs.c
@@ -338,6 +338,7 @@ static void list_bad_blocks(ext2_filsys fs, int dump)
if (retval) {
com_err("ext2fs_badblocks_list_iterate_begin", retval,
"%s", _("while printing bad block list"));
+ ext2fs_badblocks_list_free(bb_list);
return;
}
if (dump) {
diff --git a/resize/resize2fs.c b/resize/resize2fs.c
index b9783e8c..3b9b1ed1 100644
--- a/resize/resize2fs.c
+++ b/resize/resize2fs.c
@@ -1781,11 +1781,11 @@ static errcode_t block_mover(ext2_resize_t rfs)
fs->inode_blocks_per_group,
&rfs->itable_buf);
if (retval)
- return retval;
+ goto errout;
}
retval = ext2fs_create_extent_table(&rfs->bmap, 0);
if (retval)
- return retval;
+ goto errout;
/*
* The first step is to figure out where all of the blocks
--
2.27.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 5/6] e2fsck: check whether ldesc is null before accessing, it in end_problem_latch()
2021-12-31 7:40 [PATCH v2 0/6] solve memory leak and check whether NULL pointer zhanchengbin
` (3 preceding siblings ...)
2021-12-31 7:42 ` [PATCH v2 4/6] e2fsprogs: call ext2fs_badblocks_list_free() to free, list in exception branch zhanchengbin
@ 2021-12-31 7:43 ` zhanchengbin
2021-12-31 7:43 ` [PATCH v2 6/6] lib/ext2fs: call ext2fs_free_mem() to free &io->name, in exception branch zhanchengbin
2022-05-12 17:14 ` [PATCH v2 0/6] solve memory leak and check whether NULL pointer Theodore Ts'o
6 siblings, 0 replies; 8+ messages in thread
From: zhanchengbin @ 2021-12-31 7:43 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: linux-ext4, liuzhiqiang26, linfeilong, wubo40
In end_problem_latch(), ldesc need check not NULL before dereference.
Signed-off-by: zhanchengbin <zhanchengbin1@huawei.com>
---
e2fsck/problem.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/e2fsck/problem.c b/e2fsck/problem.c
index f454dcb7..fd814f9e 100644
--- a/e2fsck/problem.c
+++ b/e2fsck/problem.c
@@ -2394,6 +2394,8 @@ int end_problem_latch(e2fsck_t ctx, int mask)
int answer = -1;
ldesc = find_latch(mask);
+ if (!ldesc)
+ return answer;
if (ldesc->end_message && (ldesc->flags & PRL_LATCHED)) {
clear_problem_context(&pctx);
answer = fix_problem(ctx, ldesc->end_message, &pctx);
--
2.27.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 6/6] lib/ext2fs: call ext2fs_free_mem() to free &io->name, in exception branch
2021-12-31 7:40 [PATCH v2 0/6] solve memory leak and check whether NULL pointer zhanchengbin
` (4 preceding siblings ...)
2021-12-31 7:43 ` [PATCH v2 5/6] e2fsck: check whether ldesc is null before accessing, it in end_problem_latch() zhanchengbin
@ 2021-12-31 7:43 ` zhanchengbin
2022-05-12 17:14 ` [PATCH v2 0/6] solve memory leak and check whether NULL pointer Theodore Ts'o
6 siblings, 0 replies; 8+ messages in thread
From: zhanchengbin @ 2021-12-31 7:43 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: linux-ext4, liuzhiqiang26, linfeilong, wubo40
In the exception branch,if we donot call ext2fs_free_mem() to
free &io->name, memory leak will occur.
Signed-off-by: zhanchengbin <zhanchengbin1@huawei.com>
---
lib/ext2fs/test_io.c | 2 ++
lib/ext2fs/undo_io.c | 2 ++
2 files changed, 4 insertions(+)
diff --git a/lib/ext2fs/test_io.c b/lib/ext2fs/test_io.c
index 480e68fc..6843edbc 100644
--- a/lib/ext2fs/test_io.c
+++ b/lib/ext2fs/test_io.c
@@ -248,6 +248,8 @@ static errcode_t test_open(const char *name, int
flags, io_channel *channel)
return 0;
cleanup:
+ if (io && io->name)
+ ext2fs_free_mem(&io->name);
if (io)
ext2fs_free_mem(&io);
if (data)
diff --git a/lib/ext2fs/undo_io.c b/lib/ext2fs/undo_io.c
index eb56f53d..0d4915cb 100644
--- a/lib/ext2fs/undo_io.c
+++ b/lib/ext2fs/undo_io.c
@@ -788,6 +788,8 @@ cleanup:
free(data->tdb_file);
if (data && data->real)
io_channel_close(data->real);
+ if (io && io->name)
+ ext2fs_free_mem(&io->name);
if (data)
ext2fs_free_mem(&data);
if (io)
--
2.27.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 0/6] solve memory leak and check whether NULL pointer
2021-12-31 7:40 [PATCH v2 0/6] solve memory leak and check whether NULL pointer zhanchengbin
` (5 preceding siblings ...)
2021-12-31 7:43 ` [PATCH v2 6/6] lib/ext2fs: call ext2fs_free_mem() to free &io->name, in exception branch zhanchengbin
@ 2022-05-12 17:14 ` Theodore Ts'o
6 siblings, 0 replies; 8+ messages in thread
From: Theodore Ts'o @ 2022-05-12 17:14 UTC (permalink / raw)
To: zhanchengbin; +Cc: linux-ext4, liuzhiqiang26, linfeilong, wubo40
On Fri, Dec 31, 2021 at 03:40:41PM +0800, zhanchengbin wrote:
> Solve the memory leak of the abnormal branch and the new null pointer check
Applied, but the patches were all white-space damaged so I had to
apply them by hand. I also reworded the commit description to be
clearer.
The one exception is the patch to lib/ss, which had already been fixed
commit a282671a0 ("libss: fix possible NULL pointer dereference on
allocation failure") in my tree.
Cheers,
- Ted
> Changes from V1:
> ---------------
> - In the V1 of the patch series, have a bug in patch 1/6, when s->s get
> memory successd, s-len is not assigned a value.
>
> zhanchengbin (6):
> e2fsck: set s->len=0 if malloc() fails in alloc_string()
> lib/ss: check whether argp is null before accessing it in
> ss_execute_command()
> lib/support: check whether inump is null before accessing it in
> quota_set_sb_inum()
> e2fsprogs: call ext2fs_badblocks_list_free() to free list in exception
> branch
> e2fsck: check whether ldesc is null before accessing it in
> end_problem_latch()
> lib/ext2fs: call ext2fs_free_mem() to free &io->name in exception
> branch
>
> e2fsck/logfile.c | 2 +-
> e2fsck/problem.c | 2 ++
> lib/ext2fs/test_io.c | 2 ++
> lib/ext2fs/undo_io.c | 2 ++
> lib/ss/execute_cmd.c | 2 ++
> lib/support/mkquota.c | 3 ++-
> misc/dumpe2fs.c | 1 +
> resize/resize2fs.c | 4 ++--
> 8 files changed, 14 insertions(+), 4 deletions(-)
>
> --
> 2.27.0
^ permalink raw reply [flat|nested] 8+ messages in thread