[v2,2/2] hfsplus: fix uninit-value in hfsplus_delete_cat()

  Syzkaller reported BUG as follows:

  =====================================================
  BUG: KMSAN: uninit-value in hfsplus_subfolders_dec
                              fs/hfsplus/catalog.c:248 [inline]
  BUG: KMSAN: uninit-value in hfsplus_delete_cat+0x1207/0x14d0
                              fs/hfsplus/catalog.c:419
   hfsplus_subfolders_dec fs/hfsplus/catalog.c:248 [inline]
   hfsplus_delete_cat+0x1207/0x14d0 fs/hfsplus/catalog.c:419
   hfsplus_rmdir+0x141/0x3d0 fs/hfsplus/dir.c:425
   hfsplus_rename+0x102/0x2e0 fs/hfsplus/dir.c:545
   vfs_rename+0x1e4c/0x2800 fs/namei.c:4779
   do_renameat2+0x173d/0x1dc0 fs/namei.c:4930
   __do_sys_renameat2 fs/namei.c:4963 [inline]
   __se_sys_renameat2 fs/namei.c:4960 [inline]
   __ia32_sys_renameat2+0x14b/0x1f0 fs/namei.c:4960
   do_syscall_32_irqs_on arch/x86/entry/common.c:112 [inline]
   __do_fast_syscall_32+0xa2/0x100 arch/x86/entry/common.c:178
   do_fast_syscall_32+0x33/0x70 arch/x86/entry/common.c:203
   do_SYSENTER_32+0x1b/0x20 arch/x86/entry/common.c:246
   entry_SYSENTER_compat_after_hwframe+0x70/0x82

  Uninit was stored to memory at:
   hfsplus_subfolders_inc fs/hfsplus/catalog.c:232 [inline]
   hfsplus_create_cat+0x19e3/0x19f0 fs/hfsplus/catalog.c:314
   hfsplus_mknod+0x1fd/0x560 fs/hfsplus/dir.c:494
   hfsplus_mkdir+0x54/0x60 fs/hfsplus/dir.c:529
   vfs_mkdir+0x62a/0x870 fs/namei.c:4036
   do_mkdirat+0x466/0x7b0 fs/namei.c:4061
   __do_sys_mkdirat fs/namei.c:4076 [inline]
   __se_sys_mkdirat fs/namei.c:4074 [inline]
   __ia32_sys_mkdirat+0xc4/0x120 fs/namei.c:4074
   do_syscall_32_irqs_on arch/x86/entry/common.c:112 [inline]
   __do_fast_syscall_32+0xa2/0x100 arch/x86/entry/common.c:178
   do_fast_syscall_32+0x33/0x70 arch/x86/entry/common.c:203
   do_SYSENTER_32+0x1b/0x20 arch/x86/entry/common.c:246
   entry_SYSENTER_compat_after_hwframe+0x70/0x82

  Uninit was created at:
   __alloc_pages+0x9f1/0xe80 mm/page_alloc.c:5581
   alloc_pages+0xaae/0xd80 mm/mempolicy.c:2285
   alloc_slab_page mm/slub.c:1794 [inline]
   allocate_slab+0x1b5/0x1010 mm/slub.c:1939
   new_slab mm/slub.c:1992 [inline]
   ___slab_alloc+0x10c3/0x2d60 mm/slub.c:3180
   __slab_alloc mm/slub.c:3279 [inline]
   slab_alloc_node mm/slub.c:3364 [inline]
   slab_alloc mm/slub.c:3406 [inline]
   __kmem_cache_alloc_lru mm/slub.c:3413 [inline]
   kmem_cache_alloc_lru+0x6f3/0xb30 mm/slub.c:3429
   alloc_inode_sb include/linux/fs.h:3125 [inline]
   hfsplus_alloc_inode+0x56/0xc0 fs/hfsplus/super.c:627
   alloc_inode+0x83/0x440 fs/inode.c:259
   iget_locked+0x2a1/0xe20 fs/inode.c:1286
   hfsplus_iget+0x5f/0xb60 fs/hfsplus/super.c:64
   hfsplus_btree_open+0x13b/0x1cf0 fs/hfsplus/btree.c:150
   hfsplus_fill_super+0x12b0/0x2a80 fs/hfsplus/super.c:473
   mount_bdev+0x508/0x840 fs/super.c:1401
   hfsplus_mount+0x49/0x60 fs/hfsplus/super.c:641
   legacy_get_tree+0x10c/0x280 fs/fs_context.c:610
   vfs_get_tree+0xa1/0x500 fs/super.c:1531
   do_new_mount+0x694/0x1580 fs/namespace.c:3040
   path_mount+0x71a/0x1eb0 fs/namespace.c:3370
   do_mount fs/namespace.c:3383 [inline]
   __do_sys_mount fs/namespace.c:3591 [inline]
   __se_sys_mount+0x734/0x840 fs/namespace.c:3568
   __ia32_sys_mount+0xdf/0x140 fs/namespace.c:3568
   do_syscall_32_irqs_on arch/x86/entry/common.c:112 [inline]
   __do_fast_syscall_32+0xa2/0x100 arch/x86/entry/common.c:178
   do_fast_syscall_32+0x33/0x70 arch/x86/entry/common.c:203
   do_SYSENTER_32+0x1b/0x20 arch/x86/entry/common.c:246
   entry_SYSENTER_compat_after_hwframe+0x70/0x82
  =====================================================

Fix this by initializing 'subfolders' of 'struct hfsplus_inode_info'
in hfsplus_iget().

Link: https://syzkaller.appspot.com/bug?id=981f82f21b973f2f5663dfea581ff8cee1ddfef2
Suggested-by: Viacheslav Dubeyko <slava@dubeyko.com>
Signed-off-by: ChenXiaoSong <chenxiaosong2@huawei.com>
---
 fs/hfsplus/super.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)
  

> On Dec 15, 2022, at 12:18 AM, ChenXiaoSong <chenxiaosong2@huawei.com> wrote:
> 
> Syzkaller reported BUG as follows:
> 
>  =====================================================
>  BUG: KMSAN: uninit-value in hfsplus_subfolders_dec
>                              fs/hfsplus/catalog.c:248 [inline]
>  BUG: KMSAN: uninit-value in hfsplus_delete_cat+0x1207/0x14d0
>                              fs/hfsplus/catalog.c:419
>   hfsplus_subfolders_dec fs/hfsplus/catalog.c:248 [inline]
>   hfsplus_delete_cat+0x1207/0x14d0 fs/hfsplus/catalog.c:419
>   hfsplus_rmdir+0x141/0x3d0 fs/hfsplus/dir.c:425
>   hfsplus_rename+0x102/0x2e0 fs/hfsplus/dir.c:545
>   vfs_rename+0x1e4c/0x2800 fs/namei.c:4779
>   do_renameat2+0x173d/0x1dc0 fs/namei.c:4930
>   __do_sys_renameat2 fs/namei.c:4963 [inline]
>   __se_sys_renameat2 fs/namei.c:4960 [inline]
>   __ia32_sys_renameat2+0x14b/0x1f0 fs/namei.c:4960
>   do_syscall_32_irqs_on arch/x86/entry/common.c:112 [inline]
>   __do_fast_syscall_32+0xa2/0x100 arch/x86/entry/common.c:178
>   do_fast_syscall_32+0x33/0x70 arch/x86/entry/common.c:203
>   do_SYSENTER_32+0x1b/0x20 arch/x86/entry/common.c:246
>   entry_SYSENTER_compat_after_hwframe+0x70/0x82
> 
>  Uninit was stored to memory at:
>   hfsplus_subfolders_inc fs/hfsplus/catalog.c:232 [inline]
>   hfsplus_create_cat+0x19e3/0x19f0 fs/hfsplus/catalog.c:314
>   hfsplus_mknod+0x1fd/0x560 fs/hfsplus/dir.c:494
>   hfsplus_mkdir+0x54/0x60 fs/hfsplus/dir.c:529
>   vfs_mkdir+0x62a/0x870 fs/namei.c:4036
>   do_mkdirat+0x466/0x7b0 fs/namei.c:4061
>   __do_sys_mkdirat fs/namei.c:4076 [inline]
>   __se_sys_mkdirat fs/namei.c:4074 [inline]
>   __ia32_sys_mkdirat+0xc4/0x120 fs/namei.c:4074
>   do_syscall_32_irqs_on arch/x86/entry/common.c:112 [inline]
>   __do_fast_syscall_32+0xa2/0x100 arch/x86/entry/common.c:178
>   do_fast_syscall_32+0x33/0x70 arch/x86/entry/common.c:203
>   do_SYSENTER_32+0x1b/0x20 arch/x86/entry/common.c:246
>   entry_SYSENTER_compat_after_hwframe+0x70/0x82
> 
>  Uninit was created at:
>   __alloc_pages+0x9f1/0xe80 mm/page_alloc.c:5581
>   alloc_pages+0xaae/0xd80 mm/mempolicy.c:2285
>   alloc_slab_page mm/slub.c:1794 [inline]
>   allocate_slab+0x1b5/0x1010 mm/slub.c:1939
>   new_slab mm/slub.c:1992 [inline]
>   ___slab_alloc+0x10c3/0x2d60 mm/slub.c:3180
>   __slab_alloc mm/slub.c:3279 [inline]
>   slab_alloc_node mm/slub.c:3364 [inline]
>   slab_alloc mm/slub.c:3406 [inline]
>   __kmem_cache_alloc_lru mm/slub.c:3413 [inline]
>   kmem_cache_alloc_lru+0x6f3/0xb30 mm/slub.c:3429
>   alloc_inode_sb include/linux/fs.h:3125 [inline]
>   hfsplus_alloc_inode+0x56/0xc0 fs/hfsplus/super.c:627
>   alloc_inode+0x83/0x440 fs/inode.c:259
>   iget_locked+0x2a1/0xe20 fs/inode.c:1286
>   hfsplus_iget+0x5f/0xb60 fs/hfsplus/super.c:64
>   hfsplus_btree_open+0x13b/0x1cf0 fs/hfsplus/btree.c:150
>   hfsplus_fill_super+0x12b0/0x2a80 fs/hfsplus/super.c:473
>   mount_bdev+0x508/0x840 fs/super.c:1401
>   hfsplus_mount+0x49/0x60 fs/hfsplus/super.c:641
>   legacy_get_tree+0x10c/0x280 fs/fs_context.c:610
>   vfs_get_tree+0xa1/0x500 fs/super.c:1531
>   do_new_mount+0x694/0x1580 fs/namespace.c:3040
>   path_mount+0x71a/0x1eb0 fs/namespace.c:3370
>   do_mount fs/namespace.c:3383 [inline]
>   __do_sys_mount fs/namespace.c:3591 [inline]
>   __se_sys_mount+0x734/0x840 fs/namespace.c:3568
>   __ia32_sys_mount+0xdf/0x140 fs/namespace.c:3568
>   do_syscall_32_irqs_on arch/x86/entry/common.c:112 [inline]
>   __do_fast_syscall_32+0xa2/0x100 arch/x86/entry/common.c:178
>   do_fast_syscall_32+0x33/0x70 arch/x86/entry/common.c:203
>   do_SYSENTER_32+0x1b/0x20 arch/x86/entry/common.c:246
>   entry_SYSENTER_compat_after_hwframe+0x70/0x82
>  =====================================================
> 
> Fix this by initializing 'subfolders' of 'struct hfsplus_inode_info'
> in hfsplus_iget().
> 
> Link: https://syzkaller.appspot.com/bug?id=981f82f21b973f2f5663dfea581ff8cee1ddfef2
> Suggested-by: Viacheslav Dubeyko <slava@dubeyko.com>
> Signed-off-by: ChenXiaoSong <chenxiaosong2@huawei.com>
> ---
> fs/hfsplus/super.c | 8 +-------
> 1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
> index 122ed89ebf9f..5812818759dd 100644
> --- a/fs/hfsplus/super.c
> +++ b/fs/hfsplus/super.c
> @@ -67,13 +67,7 @@ struct inode *hfsplus_iget(struct super_block *sb, unsigned long ino)
> 	if (!(inode->i_state & I_NEW))
> 		return inode;
> 
> -	INIT_LIST_HEAD(&HFSPLUS_I(inode)->open_dir_list);
> -	spin_lock_init(&HFSPLUS_I(inode)->open_dir_lock);
> -	mutex_init(&HFSPLUS_I(inode)->extents_lock);
> -	HFSPLUS_I(inode)->flags = 0;
> -	HFSPLUS_I(inode)->extent_state = 0;
> -	HFSPLUS_I(inode)->rsrc_inode = NULL;
> -	atomic_set(&HFSPLUS_I(inode)->opencnt, 0);
> +	hfsplus_init_inode(HFSPLUS_I(inode));
> 


Maybe, I am missing something. But where in the second version of the patch
initialization of subfolders?

Thanks,
Slava.

> 	if (inode->i_ino >= HFSPLUS_FIRSTUSER_CNID ||
> 	    inode->i_ino == HFSPLUS_ROOT_CNID) {
> -- 
> 2.31.1
>
  

在 2022/12/16 3:03, Viacheslav Dubeyko 写道:
> 
> Maybe, I am missing something. But where in the second version of the patch
> initialization of subfolders?
> 

The first patch of the patchset factor out hfsplus_init_inode() from 
hfsplus_new_inode():

void hfsplus_init_inode(struct hfsplus_inode_info *hip)
{
         INIT_LIST_HEAD(&hip->open_dir_list);
         spin_lock_init(&hip->open_dir_lock);
         mutex_init(&hip->extents_lock);
         atomic_set(&hip->opencnt, 0);
         hip->extent_state = 0;
         hip->flags = 0;
         hip->userflags = 0;
         hip->subfolders = 0; /* I am here */
         memset(hip->first_extents, 0, sizeof(hfsplus_extent_rec));
         memset(hip->cached_extents, 0, sizeof(hfsplus_extent_rec));
         hip->alloc_blocks = 0;
         hip->first_blocks = 0;
         hip->cached_start = 0;
         hip->cached_blocks = 0;
         hip->phys_size = 0;
         hip->fs_blocks = 0;
         hip->rsrc_inode = NULL;
}
  

> On Dec 15, 2022, at 5:16 PM, ChenXiaoSong <chenxiaosong2@huawei.com> wrote:
> 
> 在 2022/12/16 3:03, Viacheslav Dubeyko 写道:
>> Maybe, I am missing something. But where in the second version of the patch
>> initialization of subfolders?
> 
> The first patch of the patchset factor out hfsplus_init_inode() from hfsplus_new_inode():
> 
> void hfsplus_init_inode(struct hfsplus_inode_info *hip)
> {
>        INIT_LIST_HEAD(&hip->open_dir_list);
>        spin_lock_init(&hip->open_dir_lock);
>        mutex_init(&hip->extents_lock);
>        atomic_set(&hip->opencnt, 0);
>        hip->extent_state = 0;
>        hip->flags = 0;
>        hip->userflags = 0;
>        hip->subfolders = 0; /* I am here */
>        memset(hip->first_extents, 0, sizeof(hfsplus_extent_rec));
>        memset(hip->cached_extents, 0, sizeof(hfsplus_extent_rec));
>        hip->alloc_blocks = 0;
>        hip->first_blocks = 0;
>        hip->cached_start = 0;
>        hip->cached_blocks = 0;
>        hip->phys_size = 0;
>        hip->fs_blocks = 0;
>        hip->rsrc_inode = NULL;
> }

As far as I can see, you sent 0/2, 1/2, 2/2 patches in second version. And patch 1/2 contains
only this:

diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
index 6aa919e59483..2aa719e00ae5 100644
--- a/fs/hfsplus/hfsplus_fs.h
+++ b/fs/hfsplus/hfsplus_fs.h
@@ -472,6 +472,7 @@ extern const struct dentry_operations hfsplus_dentry_operations;

int hfsplus_write_begin(struct file *file, struct address_space *mapping,
		loff_t pos, unsigned len, struct page **pagep, void **fsdata);
+void hfsplus_init_inode(struct hfsplus_inode_info *hip);
struct inode *hfsplus_new_inode(struct super_block *sb, struct inode *dir,
				umode_t mode);
void hfsplus_delete_inode(struct inode *inode);
diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c
index 840577a0c1e7..d921b32d292e 100644
--- a/fs/hfsplus/inode.c
+++ b/fs/hfsplus/inode.c
@@ -379,22 +379,8 @@ static const struct file_operations hfsplus_file_operations = {
	.unlocked_ioctl = hfsplus_ioctl,
};

-struct inode *hfsplus_new_inode(struct super_block *sb, struct inode *dir,
-				umode_t mode)
+void hfsplus_init_inode(struct hfsplus_inode_info *hip)
{
-	struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb);
-	struct inode *inode = new_inode(sb);
-	struct hfsplus_inode_info *hip;
-
-	if (!inode)
-		return NULL;
-
-	inode->i_ino = sbi->next_cnid++;
-	inode_init_owner(&init_user_ns, inode, dir, mode);
-	set_nlink(inode, 1);
-	inode->i_mtime = inode->i_atime = inode->i_ctime = current_time(inode);
-
-	hip = HFSPLUS_I(inode);
	INIT_LIST_HEAD(&hip->open_dir_list);
	spin_lock_init(&hip->open_dir_lock);
	mutex_init(&hip->extents_lock);
@@ -412,6 +398,25 @@ struct inode *hfsplus_new_inode(struct super_block *sb, struct inode *dir,
	hip->phys_size = 0;
	hip->fs_blocks = 0;
	hip->rsrc_inode = NULL;
+}
+
+struct inode *hfsplus_new_inode(struct super_block *sb, struct inode *dir,
+				umode_t mode)
+{
+	struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb);
+	struct inode *inode = new_inode(sb);
+	struct hfsplus_inode_info *hip;
+
+	if (!inode)
+		return NULL;
+
+	inode->i_ino = sbi->next_cnid++;
+	inode_init_owner(&init_user_ns, inode, dir, mode);
+	set_nlink(inode, 1);
+	inode->i_mtime = inode->i_atime = inode->i_ctime = current_time(inode);
+
+	hip = HFSPLUS_I(inode);
+	hfsplus_init_inode(hip);
	if (S_ISDIR(inode->i_mode)) {
		inode->i_size = 2;
		sbi->folder_count++;
  

在 2022/12/17 2:09, Viacheslav Dubeyko 写道>
> So, where is here hip->subfolders = 0; /* I am here */? Sorry, maybe I missed some email.
> 

1/2 patch do not show more detail about 'hip->subfolders ', you can 
apply the patchset to baseline and use '--unified' option to 'git show' 
more detail:

```shell
git am '1/2 of the patchset' # to baseline
git show 'commit of 1/2 patch' --unified=16 # specify 16 lines of 
context instead of the default 3 lines
```

then you can see more detail as follows:

```shell
...
-struct inode *hfsplus_new_inode(struct super_block *sb, struct inode *dir,
-                               umode_t mode)
+void hfsplus_init_inode(struct hfsplus_inode_info *hip)
  {
-       struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb);
-       struct inode *inode = new_inode(sb);
-       struct hfsplus_inode_info *hip;
-
-       if (!inode)
-               return NULL;
-
-       inode->i_ino = sbi->next_cnid++;
-       inode_init_owner(&init_user_ns, inode, dir, mode);
-       set_nlink(inode, 1);
-       inode->i_mtime = inode->i_atime = inode->i_ctime = 
current_time(inode);
-
-       hip = HFSPLUS_I(inode);
         INIT_LIST_HEAD(&hip->open_dir_list);
         spin_lock_init(&hip->open_dir_lock);
         mutex_init(&hip->extents_lock);
         atomic_set(&hip->opencnt, 0);
         hip->extent_state = 0;
         hip->flags = 0;
         hip->userflags = 0;
         hip->subfolders = 0; /********* I am here *****************/
         memset(hip->first_extents, 0, sizeof(hfsplus_extent_rec));
         memset(hip->cached_extents, 0, sizeof(hfsplus_extent_rec));
         hip->alloc_blocks = 0;
         hip->first_blocks = 0;
         hip->cached_start = 0;
         hip->cached_blocks = 0;
         hip->phys_size = 0;
         hip->fs_blocks = 0;
         hip->rsrc_inode = NULL;
+}
+
+struct inode *hfsplus_new_inode(struct super_block *sb, struct inode *dir,
+                               umode_t mode)
+{
+       struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb);
+       struct inode *inode = new_inode(sb);
+       struct hfsplus_inode_info *hip;
+
+       if (!inode)
+               return NULL;
+
+       inode->i_ino = sbi->next_cnid++;
+       inode_init_owner(&init_user_ns, inode, dir, mode);
+       set_nlink(inode, 1);
+       inode->i_mtime = inode->i_atime = inode->i_ctime = 
current_time(inode);
+
+       hip = HFSPLUS_I(inode);
+       hfsplus_init_inode(hip);
         if (S_ISDIR(inode->i_mode)) {
                 inode->i_size = 2;
                 sbi->folder_count++;
                 inode->i_op = &hfsplus_dir_inode_operations;
...
```