Solve the memory leak of the abnormal branch and the new null pointer check
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, 15 insertions(+), 3 deletions(-)
--
2.23.0
In alloc_string(), when malloc fails, len in the
string structure should be 0.
Signed-off-by: zhanchengbin <[email protected]>
Signed-off-by: Zhiqiang Liu <[email protected]>
---
e2fsck/logfile.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/e2fsck/logfile.c b/e2fsck/logfile.c
index 63e9a12f..f2227ad5 100644
--- a/e2fsck/logfile.c
+++ b/e2fsck/logfile.c
@@ -32,6 +32,8 @@ static void alloc_string(struct string *s, int len)
{
s->s = malloc(len);
/* e2fsck_allocate_memory(ctx, len, "logfile name"); */
+ if (s->s == NULL)
+ s->len = 0;
s->len = len;
s->end = 0;
}
--
2.23.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]>
Signed-off-by: Zhiqiang Liu <[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.23.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]>
Signed-off-by: Zhiqiang Liu <[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.23.0
In the exception branch, not call ext2fs_badblocks_list_free() to
free list, it will cause memory leak.
Signed-off-by: zhanchengbin <[email protected]>
Signed-off-by: Zhiqiang Liu <[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 73174be0..740cec48 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.23.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.23.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]>
Signed-off-by: Zhiqiang Liu <[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.23.0
On 2021/12/3 14:26, zhanchengbin wrote:
> In alloc_string(), when malloc fails, len in the
> string structure should be 0.
>
> Signed-off-by: zhanchengbin <[email protected]>
> Signed-off-by: Zhiqiang Liu <[email protected]>
> ---
> e2fsck/logfile.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/e2fsck/logfile.c b/e2fsck/logfile.c
> index 63e9a12f..f2227ad5 100644
> --- a/e2fsck/logfile.c
> +++ b/e2fsck/logfile.c
> @@ -32,6 +32,8 @@ static void alloc_string(struct string *s, int len)
> {
> s->s = malloc(len);
> /* e2fsck_allocate_memory(ctx, len, "logfile name"); */
> + if (s->s == NULL)
> + s->len = 0;
we should add 'else' branch here.
+ if (s->s == NULL)
+ s->len = 0;
+ else
+ s->len = len;
please correct that in the v2 patches.
>
> s->end = 0;
> }