| Message ID | 20221215081820.948990-3-chenxiaosong2@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | initialize 'struct hfsplus_inode_info' in hfsplus_iget() | expand |
> 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; ... ```
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)); if (inode->i_ino >= HFSPLUS_FIRSTUSER_CNID || inode->i_ino == HFSPLUS_ROOT_CNID) {
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(-)