Solve the memory leak of the abnormal branch and the new null pointer check
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
In alloc_string(), when malloc fails, len in the
string structure should be 0.
Signed-off-by: zhanchengbin <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
In end_problem_latch(), ldesc need check not NULL before dereference.
Signed-off-by: zhanchengbin <[email protected]>
---
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
In the exception branch,if we donot call ext2fs_free_mem() to
free &io->name, memory leak will occur.
Signed-off-by: zhanchengbin <[email protected]>
---
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
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