linux-kernel - [PATCH] hfs: zero-allocate ptr and handle null root tree to mitigate KMSAN bug

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <20241022175624.1561349-2-gianf.trad@gmail.com>
Date: Tue, 22 Oct 2024 19:56:25 +0200
From: Gianfranco Trad <gianf.trad@...il.com>
To: brauner@...nel.org,
	josef@...icpanda.com
Cc: linux-fsdevel@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	skhan@...uxfoundation.org,
	Gianfranco Trad <gianf.trad@...il.com>,
	syzbot+d395b0c369e492a17530@...kaller.appspotmail.com
Subject: [PATCH] hfs: zero-allocate ptr and handle null root tree to mitigate KMSAN bug

Syzbot reports a KMSAN uninit-value bug in __hfs_cache_extent [1].

Crash report is as such:

loop0: detected capacity change from 0 to 64
=====================================================
BUG: KMSAN: uninit-value in __hfs_ext_read_extent fs/hfs/extent.c:160 [inline]
BUG: KMSAN: uninit-value in __hfs_ext_cache_extent+0x69f/0x7e0 fs/hfs/extent.c:179
 __hfs_ext_read_extent fs/hfs/extent.c:160 [inline]
 __hfs_ext_cache_extent+0x69f/0x7e0 fs/hfs/extent.c:179
 hfs_ext_read_extent fs/hfs/extent.c:202 [inline]
 hfs_get_block+0x733/0xf50 fs/hfs/extent.c:366
 __block_write_begin_int+0xa6b/0x2f80 fs/buffer.c:2121

Which comes from ptr not being zero-initialized before assigning it
to fd->search_key. Hence, use kzalloc in hfs_find_init().

However, this is not enough as by re-running reproducer the following
crash report is produced:

loop0: detected capacity change from 0 to 64
=====================================================
BUG: KMSAN: uninit-value in __hfs_ext_read_extent fs/hfs/extent.c:163 [inline]
BUG: KMSAN: uninit-value in __hfs_ext_cache_extent+0x779/0x7e0 fs/hfs/extent.c:179
 __hfs_ext_read_extent fs/hfs/extent.c:163 [inline]
 __hfs_ext_cache_extent+0x779/0x7e0 fs/hfs/extent.c:179
[...]
Local variable fd.i created at:
hfs_ext_read_extent fs/hfs/extent.c:193 [inline]
hfs_get_block+0x295/0xf50 fs/hfs/extent.c:366
__block_write_begin_int+0xa6b/0x2f80 fs/buffer.c:2121

This condition is triggered by a non-handled escape path in
bdinf.c:__hfs_brec_find() which do not initialize the remaining fields in fd:

hfs_ext_read_extent -> __hfs_ext_read_extent() -> hfs_brec_find().

In hfs_brec_find():  !ndix branch -> -ENOENT returned 
without initializing the remaining fd fields in the 
subsequent __hfs_brec_find() helper call.

Once returning to __hfs_ext_read_extent() ensure that this escape path is
handled to mitigate use of uninit fd fields causing the KMSAN bug.

Reproducer does not trigger KMSAN bug anymore [2], but rather a 
kernel BUG at fs/hfs/inode.c:444:

default:
			BUG();
			return -EIO;

which seems unrelated to the initial KMSAN bug reported, rather to
subsequent write operations tried by the reproducer with other faulty options,
immediately raising macro BUG() instead of just returning -EIO.

[1] https://syzkaller.appspot.com/bug?extid=d395b0c369e492a17530
[2] https://syzkaller.appspot.com/x/report.txt?x=12922640580000

Reported-by: syzbot+d395b0c369e492a17530@...kaller.appspotmail.com
Signed-off-by: Gianfranco Trad <gianf.trad@...il.com>
---

 fs/hfs/bfind.c  | 2 +-
 fs/hfs/extent.c | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/hfs/bfind.c b/fs/hfs/bfind.c
index ef9498a6e88a..69f93200366d 100644
--- a/fs/hfs/bfind.c
+++ b/fs/hfs/bfind.c
@@ -18,7 +18,7 @@ int hfs_find_init(struct hfs_btree *tree, struct hfs_find_data *fd)
 
 	fd->tree = tree;
 	fd->bnode = NULL;
-	ptr = kmalloc(tree->max_key_len * 2 + 4, GFP_KERNEL);
+	ptr = kzalloc(tree->max_key_len * 2 + 4, GFP_KERNEL);
 	if (!ptr)
 		return -ENOMEM;
 	fd->search_key = ptr;
diff --git a/fs/hfs/extent.c b/fs/hfs/extent.c
index 4a0ce131e233..14fd0a7bca14 100644
--- a/fs/hfs/extent.c
+++ b/fs/hfs/extent.c
@@ -160,6 +160,8 @@ static inline int __hfs_ext_read_extent(struct hfs_find_data *fd, struct hfs_ext
 	if (fd->key->ext.FNum != fd->search_key->ext.FNum ||
 	    fd->key->ext.FkType != fd->search_key->ext.FkType)
 		return -ENOENT;
+	if (!fd->tree->root && res == -ENOENT)
+		return -ENOENT;
 	if (fd->entrylength != sizeof(hfs_extent_rec))
 		return -EIO;
 	hfs_bnode_read(fd->bnode, extent, fd->entryoffset, sizeof(hfs_extent_rec));
-- 
2.43.0