[PATCH v2 0/6] solve memory leak and check whether NULL pointer

[PATCH v2 0/6] solve memory leak and check whether NULL pointer

[zhanchengbin]

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

[PATCH v2 1/6] e2fsck: set s->len=0 if malloc() fails in, alloc_string()

[zhanchengbin]

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

[PATCH v2 2/6] lib/ss: check whether argp is null before accessing it, in ss_execute_command()

[zhanchengbin]

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

[PATCH v2 3/6] lib/support: check whether inump is null before, accessing it in quota_set_sb_inum()

[zhanchengbin]

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

[PATCH v2 4/6] e2fsprogs: call ext2fs_badblocks_list_free() to free, list in exception branch

[zhanchengbin]

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

[PATCH v2 5/6] e2fsck: check whether ldesc is null before accessing, it in end_problem_latch()

[zhanchengbin]

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

[PATCH v2 6/6] lib/ext2fs: call ext2fs_free_mem() to free &io->name, in exception branch

[zhanchengbin]

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

Re: [PATCH v2 0/6] solve memory leak and check whether NULL pointer

[Theodore Ts'o]

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