KMSAN: uninit-value in z_erofs_lz4_decompress (2)

syzbot In review syzbot 4 participants 13 messages Dec 27, 2023 – Dec 31, 2023

Subsystems (3)

VFS Library code EROFS file system

lore

Share & tools

lore

syzbot

author syzbot

2023-12-27 12:31 UTC #0 lore raw

Hello,

syzbot found the following issue on:

HEAD commit:    fbafc3e621c3 Merge tag 'for_linus' of git://git.kernel.org..
git tree:       upstream
console+strace: https://syzkaller.appspot.com/x/log.txt?x=11b0a595e80000
kernel config:  https://syzkaller.appspot.com/x/.config?x=e0c7078a6b901aa3
dashboard link: https://syzkaller.appspot.com/bug?extid=6c746eea496f34b3161d
compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=169fac19e80000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=14aafc81e80000

Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/1520f7b6daa4/disk-fbafc3e6.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/8b490af009d5/vmlinux-fbafc3e6.xz
kernel image: https://storage.googleapis.com/syzbot-assets/202ca200f4a4/bzImage-fbafc3e6.xz
mounted in repro: https://storage.googleapis.com/syzbot-assets/fcf70b38bafb/mount_0.gz

IMPORTANT: if you fix the issue, please add the following tag to the commit:

Reported-by [email protected]

loop0: detected capacity change from 0 to 16
erofs: (device loop0): mounted with root inode @ nid 36.
erofs: (device loop0): z_erofs_lz4_decompress_mem: failed to decompress -12 in[46, 4050] out[917]
=====================================================

BUG: KMSAN: uninit-value in hex_dump_to_buffer+0xae9/0x10f0 lib/hexdump.c:194
 hex_dump_to_buffer+0xae9/0x10f0 lib/hexdump.c:194
 print_hex_dump+0x13d/0x3e0 lib/hexdump.c:276
 z_erofs_lz4_decompress_mem fs/erofs/decompressor.c:252 [inline]
 z_erofs_lz4_decompress+0x257e/0x2a70 fs/erofs/decompressor.c:311
 z_erofs_decompress_pcluster fs/erofs/zdata.c:1290 [inline]
 z_erofs_decompress_queue+0x338c/0x6460 fs/erofs/zdata.c:1372
 z_erofs_runqueue+0x36cd/0x3830
 z_erofs_read_folio+0x435/0x810 fs/erofs/zdata.c:1843
 filemap_read_folio+0xce/0x370 mm/filemap.c:2323
 do_read_cache_folio+0x3b4/0x11e0 mm/filemap.c:3691
 read_cache_folio+0x60/0x80 mm/filemap.c:3723
 erofs_bread+0x286/0x6f0 fs/erofs/data.c:46
 erofs_find_target_block fs/erofs/namei.c:103 [inline]
 erofs_namei+0x2fe/0x1790 fs/erofs/namei.c:177
 erofs_lookup+0x100/0x3c0 fs/erofs/namei.c:206
 lookup_one_qstr_excl+0x233/0x520 fs/namei.c:1609
 filename_create+0x2fc/0x6d0 fs/namei.c:3876
 do_mkdirat+0x69/0x800 fs/namei.c:4121
 __do_sys_mkdirat fs/namei.c:4144 [inline]
 __se_sys_mkdirat fs/namei.c:4142 [inline]
 __x64_sys_mkdirat+0xc8/0x120 fs/namei.c:4142
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0x44/0x110 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x63/0x6b

Uninit was created at:

 __alloc_pages+0x9a4/0xe00 mm/page_alloc.c:4591
 alloc_pages_mpol+0x62b/0x9d0 mm/mempolicy.c:2133
 alloc_pages mm/mempolicy.c:2204 [inline]
 folio_alloc+0x1da/0x380 mm/mempolicy.c:2211
 filemap_alloc_folio+0xa5/0x430 mm/filemap.c:974
 do_read_cache_folio+0x163/0x11e0 mm/filemap.c:3655
 read_cache_folio+0x60/0x80 mm/filemap.c:3723
 erofs_bread+0x286/0x6f0 fs/erofs/data.c:46
 erofs_find_target_block fs/erofs/namei.c:103 [inline]
 erofs_namei+0x2fe/0x1790 fs/erofs/namei.c:177
 erofs_lookup+0x100/0x3c0 fs/erofs/namei.c:206
 lookup_one_qstr_excl+0x233/0x520 fs/namei.c:1609
 filename_create+0x2fc/0x6d0 fs/namei.c:3876
 do_mkdirat+0x69/0x800 fs/namei.c:4121
 __do_sys_mkdirat fs/namei.c:4144 [inline]
 __se_sys_mkdirat fs/namei.c:4142 [inline]
 __x64_sys_mkdirat+0xc8/0x120 fs/namei.c:4142
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0x44/0x110 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x63/0x6b

CPU: 1 PID: 5006 Comm: syz-executor342 Not tainted 6.7.0-rc7-syzkaller-00003-gfbafc3e621c3 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 11/17/2023
=====================================================

---
This report is generated by a bot. It may contain errors. See https://goo.gl/tpsmEJ for more information about syzbot. syzbot engineers can be reached at [email protected].

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.

If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title

If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.

If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)

If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report

If you want to undo deduplication, reply with:
#syz undup

Gao Xiang

2023-12-27 15:19 UTC #1 lore raw

PATCH 1/1 erofs: avoid debugging output for (de)compressed data

Syzbot reported a KMSAN warning,
erofs: (device loop0): z_erofs_lz4_decompress_mem: failed to decompress -12 in[46, 4050] out[917]
=====================================================

BUG: KMSAN: uninit-value in hex_dump_to_buffer+0xae9/0x10f0 lib/hexdump.c:194
  ..
  print_hex_dump+0x13d/0x3e0 lib/hexdump.c:276
  z_erofs_lz4_decompress_mem fs/erofs/decompressor.c:252 [inline]
  z_erofs_lz4_decompress+0x257e/0x2a70 fs/erofs/decompressor.c:311
  z_erofs_decompress_pcluster fs/erofs/zdata.c:1290 [inline]
  z_erofs_decompress_queue+0x338c/0x6460 fs/erofs/zdata.c:1372
  z_erofs_runqueue+0x36cd/0x3830
  z_erofs_read_folio+0x435/0x810 fs/erofs/zdata.c:1843

The root cause is that the printed decompressed buffer may be filled incompletely due to decompression failure. Since they were once only used for debugging, get rid of them now.

Reported-by [email protected]

Closes https://lore.kernel.org/r/[email protected] open here

Signed-off-by Gao Xiang <[email protected]>

---
 fs/erofs/decompressor.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

fs/erofs/decompressor.c +1 -7

index af98e88908ee..923afef7997a 100644
@@ -246,15 +246,9 @@ static int z_erofs_lz4_decompress_mem(struct z_erofs_lz4_decompress_ctx *ctx,
 	if (ret != rq->outputsize) {
 		erofs_err(rq->sb, "failed to decompress %d in[%u, %u] out[%u]",
 			  ret, rq->inputsize, inputmargin, rq->outputsize);
-
-		print_hex_dump(KERN_DEBUG, "[ in]: ", DUMP_PREFIX_OFFSET,
-			       16, 1, src + inputmargin, rq->inputsize, true);
-		print_hex_dump(KERN_DEBUG, "[out]: ", DUMP_PREFIX_OFFSET,
-			       16, 1, out, rq->outputsize, true);
-
 		if (ret >= 0)
 			memset(out + ret, 0, rq->outputsize - ret);
-		ret = -EIO;
+		ret = -EFSCORRUPTED;
 	} else {
 		ret = 0;
 	}

Signature · 1 line

2.39.3

Yue Hu

reviewer Reviewed-by on v1 1/1

2023-12-28 01:30 UTC #2 lore raw

Re: [PATCH] erofs: avoid debugging output for (de)compressed data

Quoted 13 lines · On Wed, 27 Dec 2023 23:19:03 +0800 Gao Xiang <[email protected]>…

Syzbot reported a KMSAN warning,
erofs: (device loop0): z_erofs_lz4_decompress_mem: failed to decompress -12 in[46, 4050] out[917]
=====================================================

BUG: KMSAN: uninit-value in hex_dump_to_buffer+0xae9/0x10f0 lib/hexdump.c:194
  ..
  print_hex_dump+0x13d/0x3e0 lib/hexdump.c:276
  z_erofs_lz4_decompress_mem fs/erofs/decompressor.c:252 [inline]
  z_erofs_lz4_decompress+0x257e/0x2a70 fs/erofs/decompressor.c:311
  z_erofs_decompress_pcluster fs/erofs/zdata.c:1290 [inline]
  z_erofs_decompress_queue+0x338c/0x6460 fs/erofs/zdata.c:1372
  z_erofs_runqueue+0x36cd/0x3830
  z_erofs_read_folio+0x435/0x810 fs/erofs/zdata.c:1843

Quoted 7 lines

The root cause is that the printed decompressed buffer may be filled incompletely due to decompression failure. Since they were once only used for debugging, get rid of them now.
Reported-by [email protected]
Closes https://lore.kernel.org/r/[email protected] open here
Signed-off-by Gao Xiang <[email protected]>

Reviewed-by Yue Hu <[email protected]>

syzbot

author syzbot

2023-12-28 01:11 UTC #3 lore raw

In reply to syzbot

For archival purposes, forwarding an incoming command email to [email protected].

***

Subject: [erofs?] KMSAN: uninit-value in z_erofs_lz4_decompress (2)
Author: [email protected]

#syz test https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git fbafc3e621c3

lib/hexdump.c +3 -1

index 06833d404398..68b30bf6c6a3 100644
@@ -263,12 +263,14 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
 		    const void *buf, size_t len, bool ascii)
 {
 	const u8 *ptr = buf;
-	int i, linelen, remaining = len;
+	int i, linelen, remaining;
 	unsigned char linebuf[32 * 3 + 2 + 32 + 1];
 
 	if (rowsize != 16 && rowsize != 32)
 		rowsize = 16;
 
+	len = len > sizeof(linebuf) ? sizeof(linebuf) : len;
+	remaining = len;
 	for (i = 0; i < len; i += rowsize) {
 		linelen = min(remaining, rowsize);
 		remaining -= rowsize;

Gao Xiang

2023-12-28 03:36 UTC #4 lore raw

In reply to syzbot

#syz test git://git.kernel.org/pub/scm/linux/kernel/git/xiang/erofs.git dev-test

syzbot

author syzbot

2023-12-28 04:23 UTC #5 lore raw

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-and-tested-by [email protected]

Tested on:

commit:         94da00a0 erofs: avoid debugging output for (de)compres..
git tree:       git://git.kernel.org/pub/scm/linux/kernel/git/xiang/erofs.git dev-test
console output: https://syzkaller.appspot.com/x/log.txt?x=13715b95e80000
kernel config:  https://syzkaller.appspot.com/x/.config?x=f711bc2a7eb1db25
dashboard link: https://syzkaller.appspot.com/bug?extid=6c746eea496f34b3161d
compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40

Note: no patches were applied.
Note: testing is done by a robot and is best-effort only.

syzbot

author syzbot

2023-12-28 05:56 UTC #6 lore raw

In reply to syzbot

For archival purposes, forwarding an incoming command email to [email protected].

***

Subject: [erofs?] KMSAN: uninit-value in z_erofs_lz4_decompress (2)
Author: [email protected]

#syz test https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git fbafc3e621c3

lib/hexdump.c +1 -1

index 06833d404398..e146b1bf73dc 100644
@@ -264,7 +264,7 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
 {
 	const u8 *ptr = buf;
 	int i, linelen, remaining = len;
-	unsigned char linebuf[32 * 3 + 2 + 32 + 1];
+	unsigned char linebuf[32 * 3 + 2 + 32 + 1] = "";
 
 	if (rowsize != 16 && rowsize != 32)
 		rowsize = 16;

syzbot

author syzbot

2023-12-28 13:54 UTC #7 lore raw

In reply to syzbot

For archival purposes, forwarding an incoming command email to [email protected].

***

Subject: Re: [syzbot] [erofs?] KMSAN: uninit-value in z_erofs_lz4_decompress (2)
Author: [email protected]

please test uninit-value in z_erofs_lz4_decompress (2)

#syz test https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git fbafc3e621c3

fs/erofs/decompressor.c +1 -1

index 021be5feb1bc..1c19731c8fc6 100644
@@ -250,7 +250,7 @@ static int z_erofs_lz4_decompress_mem(struct z_erofs_lz4_decompress_ctx *ctx,
 		print_hex_dump(KERN_DEBUG, "[ in]: ", DUMP_PREFIX_OFFSET,
 			       16, 1, src + inputmargin, rq->inputsize, true);
 		print_hex_dump(KERN_DEBUG, "[out]: ", DUMP_PREFIX_OFFSET,
-			       16, 1, out, rq->outputsize, true);
+			       16, 1, out, ret > 0 ? ret : rq->outputsize, true);
 
 		if (ret >= 0)
 			memset(out + ret, 0, rq->outputsize - ret);

syzbot

author syzbot

2023-12-28 14:08 UTC #8 lore raw

In reply to syzbot

For archival purposes, forwarding an incoming command email to [email protected].

***

Subject: Re: [syzbot] [erofs?] KMSAN: uninit-value in z_erofs_lz4_decompress (2)
Author: [email protected]

please test uninit-value in z_erofs_lz4_decompress (2)

#syz test https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git fbafc3e621c3

fs/erofs/decompressor.c +2 -1

index 021be5feb1bc..c0983c3db77f 100644
@@ -250,7 +250,8 @@ static int z_erofs_lz4_decompress_mem(struct z_erofs_lz4_decompress_ctx *ctx,
 		print_hex_dump(KERN_DEBUG, "[ in]: ", DUMP_PREFIX_OFFSET,
 			       16, 1, src + inputmargin, rq->inputsize, true);
 		print_hex_dump(KERN_DEBUG, "[out]: ", DUMP_PREFIX_OFFSET,
-			       16, 1, out, rq->outputsize, true);
+			       16, 1, out, ret < 0 ? min_t(unsigned int, 
+				       rq->outputsize, rq->inputsize) : rq->outputsize, true);
 
 		if (ret >= 0)
 			memset(out + ret, 0, rq->outputsize - ret);

syzbot

author syzbot

2023-12-28 22:12 UTC #9 lore raw

In reply to syzbot

For archival purposes, forwarding an incoming command email to [email protected].

***

Subject: Re: [syzbot] [erofs?] KMSAN: uninit-value in z_erofs_lz4_decompress (2)
Author: [email protected]

please test uninit-value in z_erofs_lz4_decompress (2)

#syz test https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git fbafc3e621c3

fs/erofs/decompressor.c +2 -1

index 021be5feb1bc..f4cc77e3255f 100644
@@ -250,7 +250,8 @@ static int z_erofs_lz4_decompress_mem(struct z_erofs_lz4_decompress_ctx *ctx,
 		print_hex_dump(KERN_DEBUG, "[ in]: ", DUMP_PREFIX_OFFSET,
 			       16, 1, src + inputmargin, rq->inputsize, true);
 		print_hex_dump(KERN_DEBUG, "[out]: ", DUMP_PREFIX_OFFSET,
-			       16, 1, out, rq->outputsize, true);
+			       16, 1, out, ret < 0 ? (ret + rq->inputsize) : 
+			       rq->outputsize, true);
 
 		if (ret >= 0)
 			memset(out + ret, 0, rq->outputsize - ret);

Edward Adam Davis

2023-12-29 11:14 UTC #10 lore raw

PATCH 1/1 erofs: fix uninit-value in z_erofs_lz4_decompress

In reply to syzbot

When LZ4 decompression fails, the number of bytes read from out should be inputsize plus the returned overflow value ret.

Reported-and-tested-by [email protected]

Signed-off-by Edward Adam Davis <[email protected]>

---
 fs/erofs/decompressor.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

fs/erofs/decompressor.c +2 -1

index 021be5feb1bc..8ac3f96676c4 100644
@@ -250,7 +250,8 @@ static int z_erofs_lz4_decompress_mem(struct z_erofs_lz4_decompress_ctx *ctx,
 		print_hex_dump(KERN_DEBUG, "[ in]: ", DUMP_PREFIX_OFFSET,
 			       16, 1, src + inputmargin, rq->inputsize, true);
 		print_hex_dump(KERN_DEBUG, "[out]: ", DUMP_PREFIX_OFFSET,
-			       16, 1, out, rq->outputsize, true);
+			       16, 1, out, (ret < 0 && rq->inputsize > 0) ? 
+			       (ret + rq->inputsize) : rq->outputsize, true);
 
 		if (ret >= 0)
 			memset(out + ret, 0, rq->outputsize - ret);

Signature · 1 line

2.43.0

Gao Xiang

2023-12-31 01:14 UTC #11 lore raw

Re: [PATCH] erofs: fix uninit-value in z_erofs_lz4_decompress

Quoted 14 lines · Edward Adam Davis

When LZ4 decompression fails, the number of bytes read from out should be inputsize plus the returned overflow value ret.
Reported-and-tested-by [email protected]
Signed-off-by Edward Adam Davis <[email protected]>
---
  fs/erofs/decompressor.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)
fs/erofs/decompressor.c +0 -0
index 021be5feb1bc..8ac3f96676c4 100644

fs/erofs/decompressor.c static int z_erofs_lz4_decompress_mem(struct z_erofs_lz4_decompress_ctx *ctx, · line 254

Quoted 6 lines

fs/erofs/decompressor.c +2 -1

@@ -250,7 +250,8 @@ static int z_erofs_lz4_decompress_mem(struct z_erofs_lz4_decompress_ctx *ctx,
  		print_hex_dump(KERN_DEBUG, "[ in]: ", DUMP_PREFIX_OFFSET,
  			       16, 1, src + inputmargin, rq->inputsize, true);
  		print_hex_dump(KERN_DEBUG, "[out]: ", DUMP_PREFIX_OFFSET,
-			       16, 1, out, rq->outputsize, true);
+			       16, 1, out, (ret < 0 && rq->inputsize > 0) ?
+			       (ret + rq->inputsize) : rq->outputsize, true);

It's incorrect since output decompressed buffer has no relationship with rq->inputsize and ret + rq->inputsize is meaningless too.

Also, the issue was already fixed by avoiding debugging messages as
https://lore.kernel.org/r/[email protected]

Thanks,
Gao Xiang

Edward Adam Davis

2023-12-31 02:32 UTC #12 lore raw

Re: [PATCH] erofs: fix uninit-value in z_erofs_lz4_decompress

Quoted 9 lines · Gao Xiang

Quoted 9 lines
When LZ4 decompression fails, the number of bytes read from out should be inputsize plus the returned overflow value ret.
Reported-and-tested-by [email protected]
Signed-off-by Edward Adam Davis <[email protected]>
---
  fs/erofs/decompressor.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

Quoted 14 lines

fs/erofs/decompressor.c static int z_erofs_lz4_decompress_mem(struct z_erofs_lz4_decompress_ctx *ctx, · line 254
Quoted 11 lines
fs/erofs/decompressor.c +2 -1
index 021be5feb1bc..8ac3f96676c4 100644
@@ -250,7 +250,8 @@ static int z_erofs_lz4_decompress_mem(struct z_erofs_lz4_decompress_ctx *ctx,
  		print_hex_dump(KERN_DEBUG, "[ in]: ", DUMP_PREFIX_OFFSET,
  			       16, 1, src + inputmargin, rq->inputsize, true);
  		print_hex_dump(KERN_DEBUG, "[out]: ", DUMP_PREFIX_OFFSET,
-			       16, 1, out, rq->outputsize, true);
+			       16, 1, out, (ret < 0 && rq->inputsize > 0) ?
+			       (ret + rq->inputsize) : rq->outputsize, true);
It's incorrect since output decompressed buffer has no relationship with rq->inputsize and ret + rq->inputsize is meaningless too.

In this case, the value of ret is -12.
When LZ4_decompress_generic() fails, it will return "return (int) (- ((const char *) ip) - src) -1;"

Therefore, it can be clearly stated that the decompression has been carried out to the 11 bytes of src, so reading the value of the first 11 bytes of out is effective. Therefore, my patch should be more accurate as follows:

-			       16, 1, out, rq->outputsize, true);
+			       16, 1, out, (ret < 0 && rq->inputsize > 0) ?
+			       (0 - ret) : rq->outputsize, true);

Quoted 3 lines

Also, the issue was already fixed by avoiding debugging messages as
https://lore.kernel.org/r/[email protected]

This just deleted the output.

BR,
Edward