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

[PATCH 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

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

[PATCH 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>
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
---
  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

[PATCH 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>
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@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.23.0

[PATCH 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>
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@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.23.0

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

[zhanchengbin]

In the exception branch, not call ext2fs_badblocks_list_free() to
free list, it will cause memory leak.

Signed-off-by: zhanchengbin <zhanchengbin1@huawei.com>
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@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 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

[PATCH 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.23.0

[PATCH 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>
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@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.23.0

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

[Zhiqiang Liu]

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 <zhanchengbin1@huawei.com>
> Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
> ---
>  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;
>  }