diff mbox series

[v2] Valgrind reported error messages like following:

Message ID 1592533574-24249-1-git-send-email-wangshilong1991@gmail.com
State New
Headers show
Series [v2] Valgrind reported error messages like following: | expand

Commit Message

Wang Shilong June 19, 2020, 2:26 a.m. UTC
From: Wang Shilong <wshilong@ddn.com>

==129205==  Address 0x1b804b04 is 4 bytes after a block of size 4,096 alloc'd
==129205==    at 0x483980B: malloc (vg_replace_malloc.c:307)
==129205==    by 0x44F973: ext2fs_get_mem (ext2fs.h:1846)
==129205==    by 0x44F973: ext2fs_get_pathname (get_pathname.c:162)
==129205==    by 0x430917: print_pathname (message.c:212)
==129205==    by 0x430FB1: expand_percent_expression (message.c:462)
==129205==    by 0x430FB1: print_e2fsck_message (message.c:544)
==129205==    by 0x430BED: expand_at_expression (message.c:262)
==129205==    by 0x430BED: print_e2fsck_message (message.c:528)
==129205==    by 0x430450: fix_problem (problem.c:2494)
==129205==    by 0x423F8B: e2fsck_process_bad_inode (pass2.c:1929)
==129205==    by 0x425AE8: check_dir_block (pass2.c:1407)
==129205==    by 0x426942: check_dir_block2 (pass2.c:961)
==129205==    by 0x445736: ext2fs_dblist_iterate3.part.0 (dblist.c:254)
==129205==    by 0x423835: e2fsck_pass2 (pass2.c:187)
==129205==    by 0x414B19: e2fsck_run (e2fsck.c:257)

Dir block might be corrupted and cause the next dirent is out
of block size boundary, even though we have the check to avoid
problem, memory check tools like valgrind still complains it.

Patch try to fix the problem by checking if offset exceed max
offset firstly before getting the pointer.

Signed-off-by: Wang Shilong <wshilong@ddn.com>
---
v1->v2:
kept same return value for corruption case as before.
---
 lib/ext2fs/csum.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

Comments

Wang Shilong June 19, 2020, 2:30 a.m. UTC | #1
Sorry, please ignore this version, patch title is wrong...

On Fri, Jun 19, 2020 at 10:26 AM Wang Shilong <wangshilong1991@gmail.com> wrote:
>
> From: Wang Shilong <wshilong@ddn.com>
>
> ==129205==  Address 0x1b804b04 is 4 bytes after a block of size 4,096 alloc'd
> ==129205==    at 0x483980B: malloc (vg_replace_malloc.c:307)
> ==129205==    by 0x44F973: ext2fs_get_mem (ext2fs.h:1846)
> ==129205==    by 0x44F973: ext2fs_get_pathname (get_pathname.c:162)
> ==129205==    by 0x430917: print_pathname (message.c:212)
> ==129205==    by 0x430FB1: expand_percent_expression (message.c:462)
> ==129205==    by 0x430FB1: print_e2fsck_message (message.c:544)
> ==129205==    by 0x430BED: expand_at_expression (message.c:262)
> ==129205==    by 0x430BED: print_e2fsck_message (message.c:528)
> ==129205==    by 0x430450: fix_problem (problem.c:2494)
> ==129205==    by 0x423F8B: e2fsck_process_bad_inode (pass2.c:1929)
> ==129205==    by 0x425AE8: check_dir_block (pass2.c:1407)
> ==129205==    by 0x426942: check_dir_block2 (pass2.c:961)
> ==129205==    by 0x445736: ext2fs_dblist_iterate3.part.0 (dblist.c:254)
> ==129205==    by 0x423835: e2fsck_pass2 (pass2.c:187)
> ==129205==    by 0x414B19: e2fsck_run (e2fsck.c:257)
>
> Dir block might be corrupted and cause the next dirent is out
> of block size boundary, even though we have the check to avoid
> problem, memory check tools like valgrind still complains it.
>
> Patch try to fix the problem by checking if offset exceed max
> offset firstly before getting the pointer.
>
> Signed-off-by: Wang Shilong <wshilong@ddn.com>
> ---
> v1->v2:
> kept same return value for corruption case as before.
> ---
>  lib/ext2fs/csum.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/lib/ext2fs/csum.c b/lib/ext2fs/csum.c
> index c2550365..417a1fba 100644
> --- a/lib/ext2fs/csum.c
> +++ b/lib/ext2fs/csum.c
> @@ -266,16 +266,14 @@ static errcode_t __get_dirent_tail(ext2_filsys fs,
>         d = dirent;
>         top = EXT2_DIRENT_TAIL(dirent, fs->blocksize);
>
> -       rec_len = translate(d->rec_len);
>         while ((void *) d < top) {
> -               if ((rec_len < 8) || (rec_len & 0x03))
> +               rec_len = translate(d->rec_len);
> +               if ((rec_len < 8) || (rec_len & 0x03) ||
> +                   (rec_len > (char *)dirent + fs->blocksize - (char *)d))
>                         return EXT2_ET_DIR_CORRUPTED;
>                 d = (struct ext2_dir_entry *)(((char *)d) + rec_len);
> -               rec_len = translate(d->rec_len);
>         }
>
> -       if ((char *)d > ((char *)dirent + fs->blocksize))
> -                       return EXT2_ET_DIR_CORRUPTED;
>         if (d != top)
>                 return EXT2_ET_DIR_NO_SPACE_FOR_CSUM;
>
> --
> 2.25.4
>
diff mbox series

Patch

diff --git a/lib/ext2fs/csum.c b/lib/ext2fs/csum.c
index c2550365..417a1fba 100644
--- a/lib/ext2fs/csum.c
+++ b/lib/ext2fs/csum.c
@@ -266,16 +266,14 @@  static errcode_t __get_dirent_tail(ext2_filsys fs,
 	d = dirent;
 	top = EXT2_DIRENT_TAIL(dirent, fs->blocksize);
 
-	rec_len = translate(d->rec_len);
 	while ((void *) d < top) {
-		if ((rec_len < 8) || (rec_len & 0x03))
+		rec_len = translate(d->rec_len);
+		if ((rec_len < 8) || (rec_len & 0x03) ||
+		    (rec_len > (char *)dirent + fs->blocksize - (char *)d))
 			return EXT2_ET_DIR_CORRUPTED;
 		d = (struct ext2_dir_entry *)(((char *)d) + rec_len);
-		rec_len = translate(d->rec_len);
 	}
 
-	if ((char *)d > ((char *)dirent + fs->blocksize))
-			return EXT2_ET_DIR_CORRUPTED;
 	if (d != top)
 		return EXT2_ET_DIR_NO_SPACE_FOR_CSUM;