| Message ID | 93ee5e5a0e35342480860317b1c3d4f5680f7e54.1713344114.git.jth@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: fix information leak in btrfs_ioctl_logical_to_ino() | expand |
在 2024/4/17 18:25, Johannes Thumshirn 写道: > From: Johannes Thumshirn <johannes.thumshirn@wdc.com> > > Syzbot reported the information leak for in btrfs_ioctl_logical_to_ino(): > > BUG: KMSAN: kernel-infoleak in instrument_copy_to_user include/linux/instrumented.h:114 [inline] > BUG: KMSAN: kernel-infoleak in _copy_to_user+0xbc/0x110 lib/usercopy.c:40 > instrument_copy_to_user include/linux/instrumented.h:114 [inline] > _copy_to_user+0xbc/0x110 lib/usercopy.c:40 > copy_to_user include/linux/uaccess.h:191 [inline] > btrfs_ioctl_logical_to_ino+0x440/0x750 fs/btrfs/ioctl.c:3499 > btrfs_ioctl+0x714/0x1260 > vfs_ioctl fs/ioctl.c:51 [inline] > __do_sys_ioctl fs/ioctl.c:904 [inline] > __se_sys_ioctl+0x261/0x450 fs/ioctl.c:890 > __x64_sys_ioctl+0x96/0xe0 fs/ioctl.c:890 > x64_sys_call+0x1883/0x3b50 arch/x86/include/generated/asm/syscalls_64.h:17 > do_syscall_x64 arch/x86/entry/common.c:52 [inline] > do_syscall_64+0xcf/0x1e0 arch/x86/entry/common.c:83 > entry_SYSCALL_64_after_hwframe+0x77/0x7f > > Uninit was created at: > __kmalloc_large_node+0x231/0x370 mm/slub.c:3921 > __do_kmalloc_node mm/slub.c:3954 [inline] > __kmalloc_node+0xb07/0x1060 mm/slub.c:3973 > kmalloc_node include/linux/slab.h:648 [inline] > kvmalloc_node+0xc0/0x2d0 mm/util.c:634 > kvmalloc include/linux/slab.h:766 [inline] > init_data_container+0x49/0x1e0 fs/btrfs/backref.c:2779 > btrfs_ioctl_logical_to_ino+0x17c/0x750 fs/btrfs/ioctl.c:3480 > btrfs_ioctl+0x714/0x1260 > vfs_ioctl fs/ioctl.c:51 [inline] > __do_sys_ioctl fs/ioctl.c:904 [inline] > __se_sys_ioctl+0x261/0x450 fs/ioctl.c:890 > __x64_sys_ioctl+0x96/0xe0 fs/ioctl.c:890 > x64_sys_call+0x1883/0x3b50 arch/x86/include/generated/asm/syscalls_64.h:17 > do_syscall_x64 arch/x86/entry/common.c:52 [inline] > do_syscall_64+0xcf/0x1e0 arch/x86/entry/common.c:83 > entry_SYSCALL_64_after_hwframe+0x77/0x7f > > Bytes 40-65535 of 65536 are uninitialized > Memory access of size 65536 starts at ffff888045a40000 > > This happens, because we're copying a 'struct btrfs_data_container' back > to user-space. This btrfs_data_container is allocated in > 'init_data_container()' via kvmalloc(), which does not zero-fill the > memory. > > Fix this by using kvzalloc() which zeroes out the memory on allocation. > > Reported-by: syzbot+510a1abbb8116eeb341d@syzkaller.appspotmail.com > Signed-off-by: Johannes Thumshirn <Johannes.thumshirn@wdc.com> Reviewed-by: Qu Wenruo <wqu@suse.com> Thanks, Qu > --- > fs/btrfs/backref.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c > index c1e6a5bbeeaf..4b993c7104fe 100644 > --- a/fs/btrfs/backref.c > +++ b/fs/btrfs/backref.c > @@ -2776,7 +2776,7 @@ struct btrfs_data_container *init_data_container(u32 total_bytes) > size_t alloc_bytes; > > alloc_bytes = max_t(size_t, total_bytes, sizeof(*data)); > - data = kvmalloc(alloc_bytes, GFP_KERNEL); > + data = kvzalloc(alloc_bytes, GFP_KERNEL); > if (!data) > return ERR_PTR(-ENOMEM); >
On Wed, Apr 17, 2024 at 9:59 AM Johannes Thumshirn <jth@kernel.org> wrote: > > From: Johannes Thumshirn <johannes.thumshirn@wdc.com> > > Syzbot reported the information leak for in btrfs_ioctl_logical_to_ino(): > > BUG: KMSAN: kernel-infoleak in instrument_copy_to_user include/linux/instrumented.h:114 [inline] > BUG: KMSAN: kernel-infoleak in _copy_to_user+0xbc/0x110 lib/usercopy.c:40 > instrument_copy_to_user include/linux/instrumented.h:114 [inline] > _copy_to_user+0xbc/0x110 lib/usercopy.c:40 > copy_to_user include/linux/uaccess.h:191 [inline] > btrfs_ioctl_logical_to_ino+0x440/0x750 fs/btrfs/ioctl.c:3499 > btrfs_ioctl+0x714/0x1260 > vfs_ioctl fs/ioctl.c:51 [inline] > __do_sys_ioctl fs/ioctl.c:904 [inline] > __se_sys_ioctl+0x261/0x450 fs/ioctl.c:890 > __x64_sys_ioctl+0x96/0xe0 fs/ioctl.c:890 > x64_sys_call+0x1883/0x3b50 arch/x86/include/generated/asm/syscalls_64.h:17 > do_syscall_x64 arch/x86/entry/common.c:52 [inline] > do_syscall_64+0xcf/0x1e0 arch/x86/entry/common.c:83 > entry_SYSCALL_64_after_hwframe+0x77/0x7f > > Uninit was created at: > __kmalloc_large_node+0x231/0x370 mm/slub.c:3921 > __do_kmalloc_node mm/slub.c:3954 [inline] > __kmalloc_node+0xb07/0x1060 mm/slub.c:3973 > kmalloc_node include/linux/slab.h:648 [inline] > kvmalloc_node+0xc0/0x2d0 mm/util.c:634 > kvmalloc include/linux/slab.h:766 [inline] > init_data_container+0x49/0x1e0 fs/btrfs/backref.c:2779 > btrfs_ioctl_logical_to_ino+0x17c/0x750 fs/btrfs/ioctl.c:3480 > btrfs_ioctl+0x714/0x1260 > vfs_ioctl fs/ioctl.c:51 [inline] > __do_sys_ioctl fs/ioctl.c:904 [inline] > __se_sys_ioctl+0x261/0x450 fs/ioctl.c:890 > __x64_sys_ioctl+0x96/0xe0 fs/ioctl.c:890 > x64_sys_call+0x1883/0x3b50 arch/x86/include/generated/asm/syscalls_64.h:17 > do_syscall_x64 arch/x86/entry/common.c:52 [inline] > do_syscall_64+0xcf/0x1e0 arch/x86/entry/common.c:83 > entry_SYSCALL_64_after_hwframe+0x77/0x7f > > Bytes 40-65535 of 65536 are uninitialized > Memory access of size 65536 starts at ffff888045a40000 > > This happens, because we're copying a 'struct btrfs_data_container' back > to user-space. This btrfs_data_container is allocated in > 'init_data_container()' via kvmalloc(), which does not zero-fill the > memory. > > Fix this by using kvzalloc() which zeroes out the memory on allocation. > > Reported-by: syzbot+510a1abbb8116eeb341d@syzkaller.appspotmail.com > Signed-off-by: Johannes Thumshirn <Johannes.thumshirn@wdc.com> > --- > fs/btrfs/backref.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c > index c1e6a5bbeeaf..4b993c7104fe 100644 > --- a/fs/btrfs/backref.c > +++ b/fs/btrfs/backref.c > @@ -2776,7 +2776,7 @@ struct btrfs_data_container *init_data_container(u32 total_bytes) > size_t alloc_bytes; > > alloc_bytes = max_t(size_t, total_bytes, sizeof(*data)); > - data = kvmalloc(alloc_bytes, GFP_KERNEL); > + data = kvzalloc(alloc_bytes, GFP_KERNEL); After this we can remove the initialization of several fields to 0 (down below, not seen in the diff), since they become redundant and make the code shorter. Thanks. > if (!data) > return ERR_PTR(-ENOMEM); > > -- > 2.35.3 > >
diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c index c1e6a5bbeeaf..4b993c7104fe 100644 --- a/fs/btrfs/backref.c +++ b/fs/btrfs/backref.c @@ -2776,7 +2776,7 @@ struct btrfs_data_container *init_data_container(u32 total_bytes) size_t alloc_bytes; alloc_bytes = max_t(size_t, total_bytes, sizeof(*data)); - data = kvmalloc(alloc_bytes, GFP_KERNEL); + data = kvzalloc(alloc_bytes, GFP_KERNEL); if (!data) return ERR_PTR(-ENOMEM);