Re: [syzbot] [hfs?] KMSAN: uninit-value in hfsplus_listxattr

* Re: [syzbot] [hfs?] KMSAN: uninit-value in hfsplus_listxattr
@ 2023-05-22 10:12 侯伟桃 Vincent Hou
  2023-05-22 10:56 ` Christian Brauner
  0 siblings, 1 reply; 3+ messages in thread
From: 侯伟桃 Vincent Hou @ 2023-05-22 10:12 UTC (permalink / raw)
  To: syzbot+92ef9ee419803871020e
  Cc: akpm, brauner, glider, keescook, linux-fsdevel, linux-kernel,
	syzkaller-bugs

Since the strbuf in hfsplus_listxattr was allocated with kmalloc and filled with hfsplus_uni2asc,
which did not fill "\0" in last byte,  in some cases,  the uninited byte may be accessed when
compare the strbuf with known namespace.  But I still need check the value of xattr in strbuf
to confirm the root cause.  Please help test with below debug patch.

#syz test: https://github.com/google/kmsan.git 80383273f7a0

--- a/fs/hfsplus/xattr.c
+++ b/fs/hfsplus/xattr.c
@@ -671,6 +671,7 @@ static ssize_t hfsplus_listxattr_finder_info(struct dentry *dentry,
 	return res;
 }
 
+extern bool kmsan_enabled;
 ssize_t hfsplus_listxattr(struct dentry *dentry, char *buffer, size_t size)
 {
 	ssize_t err;
@@ -681,6 +682,8 @@ ssize_t hfsplus_listxattr(struct dentry *dentry, char *buffer, size_t size)
 	struct hfsplus_attr_key attr_key;
 	char *strbuf;
 	int xattr_name_len;
+	int off = 0;
+	char *dumpinfo;
 
 	if ((!S_ISREG(inode->i_mode) &&
 			!S_ISDIR(inode->i_mode)) ||
@@ -705,6 +708,12 @@ ssize_t hfsplus_listxattr(struct dentry *dentry, char *buffer, size_t size)
 		res = -ENOMEM;
 		goto out;
 	}
+	dumpinfo = kzalloc(200, GFP_KERNEL);
+	if (!dumpinfo) {
+		kfree(strbuf);
+		res = -ENOMEM;
+		goto out;
+	}
 
 	err = hfsplus_find_attr(inode->i_sb, inode->i_ino, NULL, &fd);
 	if (err) {
@@ -741,6 +750,15 @@ ssize_t hfsplus_listxattr(struct dentry *dentry, char *buffer, size_t size)
 			goto end_listxattr;
 		}
 
+		pr_info("find xattr size:%ld and dump strbuf pre 20 bytes:\n", size);
+		WRITE_ONCE(kmsan_enabled, false);
+		if (kmsan_enabled == false) {
+			for (off = 0; off < 20; off++) {
+				sprintf(dumpinfo + off * 5, " 0x%02x", strbuf[off]);
+			}
+			pr_info("%s\n", dumpinfo);
+		}
+		WRITE_ONCE(kmsan_enabled, true);
 		if (!buffer || !size) {
 			if (can_list(strbuf))
 				res += name_len(strbuf, xattr_name_len);
@@ -759,6 +777,7 @@ ssize_t hfsplus_listxattr(struct dentry *dentry, char *buffer, size_t size)
 
 end_listxattr:
 	kfree(strbuf);
+	kfree(dumpinfo);
 out:
 	hfs_find_exit(&fd);
 	return res;

^ permalink raw reply	[flat|nested] 3+ messages in thread