jfs: Initialized first 8 members of the dinomap when creating imap

[PATCH V2] jfs: Initialized first 8 members of the dinomap when creating imap

Posted by Edward Adam Davis 10 months ago

syzbot reported a uninit-value in diFree. [1]

When print_hex_dump() is called to print the first 32 bytes of imap, the
first 8 members in struct dinomap are the first 32 bytes of imap, because
in_numinos, in_numfree, in_diskblock and in_maxag are not initialized when
imap is created.

When creating imap, set in_numinos, in_numfree, in_diskblock and in_maxag
to 0 to prevent this issue from happening.

[1]
BUG: KMSAN: uninit-value in hex_dump_to_buffer+0x888/0x1100 lib/hexdump.c:171
 hex_dump_to_buffer+0x888/0x1100 lib/hexdump.c:171
 print_hex_dump+0x13d/0x3e0 lib/hexdump.c:276
 diFree+0x5ba/0x4350 fs/jfs/jfs_imap.c:876
 jfs_evict_inode+0x510/0x550 fs/jfs/inode.c:156
 evict+0x723/0xd10 fs/inode.c:796
 iput_final fs/inode.c:1946 [inline]
 iput+0x97b/0xdb0 fs/inode.c:1972
 txUpdateMap+0xf3e/0x1150 fs/jfs/jfs_txnmgr.c:2367
 txLazyCommit fs/jfs/jfs_txnmgr.c:2664 [inline]
 jfs_lazycommit+0x627/0x11d0 fs/jfs/jfs_txnmgr.c:2733
 kthread+0x6b9/0xef0 kernel/kthread.c:464
 ret_from_fork+0x6d/0x90 arch/x86/kernel/process.c:148
 ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244

Uninit was created at:
 slab_post_alloc_hook mm/slub.c:4121 [inline]
 slab_alloc_node mm/slub.c:4164 [inline]
 __kmalloc_cache_noprof+0x8e3/0xdf0 mm/slub.c:4320
 kmalloc_noprof include/linux/slab.h:901 [inline]
 diMount+0x61/0x7f0 fs/jfs/jfs_imap.c:105
 jfs_mount+0xa8e/0x11d0 fs/jfs/jfs_mount.c:176
 jfs_fill_super+0xa47/0x17c0 fs/jfs/super.c:523
 get_tree_bdev_flags+0x6ec/0x910 fs/super.c:1636
 get_tree_bdev+0x37/0x50 fs/super.c:1659
 jfs_get_tree+0x34/0x40 fs/jfs/super.c:635
 vfs_get_tree+0xb1/0x5a0 fs/super.c:1814
 do_new_mount+0x71f/0x15e0 fs/namespace.c:3560
 path_mount+0x742/0x1f10 fs/namespace.c:3887
 do_mount fs/namespace.c:3900 [inline]
 __do_sys_mount fs/namespace.c:4111 [inline]
 __se_sys_mount+0x71f/0x800 fs/namespace.c:4088
 __x64_sys_mount+0xe4/0x150 fs/namespace.c:4088
 x64_sys_call+0x39bf/0x3c30 arch/x86/include/generated/asm/syscalls_64.h:166
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0xcd/0x1e0 arch/x86/entry/common.c:83

Reported-by: syzbot+df6cdcb35904203d2b6d@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=df6cdcb35904203d2b6d
Tested-by: syzbot+df6cdcb35904203d2b6d@syzkaller.appspotmail.com
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
V1 -> V2: add missing others two fields of dinomap

 fs/jfs/jfs_imap.c | 4 ++++
 fs/jfs/jfs_imap.h | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
index a360b24ed320..0cedaccb7218 100644
--- a/fs/jfs/jfs_imap.c
+++ b/fs/jfs/jfs_imap.c
@@ -134,6 +134,10 @@ int diMount(struct inode *ipimap)
 		imap->im_agctl[index].numfree =
 		    le32_to_cpu(dinom_le->in_agctl[index].numfree);
 	}
+	imap->im_diskblock = 0;
+	imap->im_maxag = 0;
+	imap->im_enuminos = 0;
+	imap->im_enumfree = 0;
 
 	/* release the buffer. */
 	release_metapage(mp);
diff --git a/fs/jfs/jfs_imap.h b/fs/jfs/jfs_imap.h
index dd7409febe28..9af1da2e4591 100644
--- a/fs/jfs/jfs_imap.h
+++ b/fs/jfs/jfs_imap.h
@@ -144,6 +144,8 @@ struct inomap {
  */
 #define	im_diskblock	im_imap.in_diskblock
 #define	im_maxag	im_imap.in_maxag
+#define	im_enuminos	im_imap.in_numinos
+#define	im_enumfree	im_imap.in_numfree
 
 extern int diFree(struct inode *);
 extern int diAlloc(struct inode *, bool, struct inode *);
-- 
2.43.0

Re: [PATCH V2] jfs: Initialized first 8 members of the dinomap when creating imap

Posted by Dave Kleikamp 10 months ago

On 2/20/25 4:56AM, Edward Adam Davis wrote:
> syzbot reported a uninit-value in diFree. [1]
> 
> When print_hex_dump() is called to print the first 32 bytes of imap, the
> first 8 members in struct dinomap are the first 32 bytes of imap, because
> in_numinos, in_numfree, in_diskblock and in_maxag are not initialized when
> imap is created.
> 
> When creating imap, set in_numinos, in_numfree, in_diskblock and in_maxag
> to 0 to prevent this issue from happening.

I appreciate the patch, but I'm accepting a different patch to fix the 
problem:

https://sourceforge.net/p/jfs/mailman/message/59132063/

Shaggy

> 
> [1]
> BUG: KMSAN: uninit-value in hex_dump_to_buffer+0x888/0x1100 lib/hexdump.c:171
>   hex_dump_to_buffer+0x888/0x1100 lib/hexdump.c:171
>   print_hex_dump+0x13d/0x3e0 lib/hexdump.c:276
>   diFree+0x5ba/0x4350 fs/jfs/jfs_imap.c:876
>   jfs_evict_inode+0x510/0x550 fs/jfs/inode.c:156
>   evict+0x723/0xd10 fs/inode.c:796
>   iput_final fs/inode.c:1946 [inline]
>   iput+0x97b/0xdb0 fs/inode.c:1972
>   txUpdateMap+0xf3e/0x1150 fs/jfs/jfs_txnmgr.c:2367
>   txLazyCommit fs/jfs/jfs_txnmgr.c:2664 [inline]
>   jfs_lazycommit+0x627/0x11d0 fs/jfs/jfs_txnmgr.c:2733
>   kthread+0x6b9/0xef0 kernel/kthread.c:464
>   ret_from_fork+0x6d/0x90 arch/x86/kernel/process.c:148
>   ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244
> 
> Uninit was created at:
>   slab_post_alloc_hook mm/slub.c:4121 [inline]
>   slab_alloc_node mm/slub.c:4164 [inline]
>   __kmalloc_cache_noprof+0x8e3/0xdf0 mm/slub.c:4320
>   kmalloc_noprof include/linux/slab.h:901 [inline]
>   diMount+0x61/0x7f0 fs/jfs/jfs_imap.c:105
>   jfs_mount+0xa8e/0x11d0 fs/jfs/jfs_mount.c:176
>   jfs_fill_super+0xa47/0x17c0 fs/jfs/super.c:523
>   get_tree_bdev_flags+0x6ec/0x910 fs/super.c:1636
>   get_tree_bdev+0x37/0x50 fs/super.c:1659
>   jfs_get_tree+0x34/0x40 fs/jfs/super.c:635
>   vfs_get_tree+0xb1/0x5a0 fs/super.c:1814
>   do_new_mount+0x71f/0x15e0 fs/namespace.c:3560
>   path_mount+0x742/0x1f10 fs/namespace.c:3887
>   do_mount fs/namespace.c:3900 [inline]
>   __do_sys_mount fs/namespace.c:4111 [inline]
>   __se_sys_mount+0x71f/0x800 fs/namespace.c:4088
>   __x64_sys_mount+0xe4/0x150 fs/namespace.c:4088
>   x64_sys_call+0x39bf/0x3c30 arch/x86/include/generated/asm/syscalls_64.h:166
>   do_syscall_x64 arch/x86/entry/common.c:52 [inline]
>   do_syscall_64+0xcd/0x1e0 arch/x86/entry/common.c:83
> 
> Reported-by: syzbot+df6cdcb35904203d2b6d@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=df6cdcb35904203d2b6d
> Tested-by: syzbot+df6cdcb35904203d2b6d@syzkaller.appspotmail.com
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> ---
> V1 -> V2: add missing others two fields of dinomap
> 
>   fs/jfs/jfs_imap.c | 4 ++++
>   fs/jfs/jfs_imap.h | 2 ++
>   2 files changed, 6 insertions(+)
> 
> diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
> index a360b24ed320..0cedaccb7218 100644
> --- a/fs/jfs/jfs_imap.c
> +++ b/fs/jfs/jfs_imap.c
> @@ -134,6 +134,10 @@ int diMount(struct inode *ipimap)
>   		imap->im_agctl[index].numfree =
>   		    le32_to_cpu(dinom_le->in_agctl[index].numfree);
>   	}
> +	imap->im_diskblock = 0;
> +	imap->im_maxag = 0;
> +	imap->im_enuminos = 0;
> +	imap->im_enumfree = 0;
>   
>   	/* release the buffer. */
>   	release_metapage(mp);
> diff --git a/fs/jfs/jfs_imap.h b/fs/jfs/jfs_imap.h
> index dd7409febe28..9af1da2e4591 100644
> --- a/fs/jfs/jfs_imap.h
> +++ b/fs/jfs/jfs_imap.h
> @@ -144,6 +144,8 @@ struct inomap {
>    */
>   #define	im_diskblock	im_imap.in_diskblock
>   #define	im_maxag	im_imap.in_maxag
> +#define	im_enuminos	im_imap.in_numinos
> +#define	im_enumfree	im_imap.in_numfree
>   
>   extern int diFree(struct inode *);
>   extern int diAlloc(struct inode *, bool, struct inode *);

Re: [PATCH V2] jfs: Initialized first 8 members of the dinomap when creating imap

Posted by Edward Adam Davis 10 months ago

> > syzbot reported a uninit-value in diFree. [1]
> > 
> > When print_hex_dump() is called to print the first 32 bytes of imap, the
> > first 8 members in struct dinomap are the first 32 bytes of imap, because
> > in_numinos, in_numfree, in_diskblock and in_maxag are not initialized when
> > imap is created.
> > 
> > When creating imap, set in_numinos, in_numfree, in_diskblock and in_maxag
> > to 0 to prevent this issue from happening.
> 
> I appreciate the patch, but I'm accepting a different patch to fix the 
> problem:
I am very disappointed with your choice. The design of "KMSAN: uninit-value X"
is used to find improper data usage and defects in the program. If you directly
use functions such as kzmalloc to clear the memory to 0, you will lose a valuable
asset--KMSAN uninit-value.

BR,
Edward