[PATCH] ntfs: move check for valid resident attribute offset and length
we should check for valid resident atribute offset and length before
loading STANDARD_INFORMATION attribute in ntfs_read_locked_inode()
let's make that check a bit earlier in the same function to avoid
use-after-free bug
Reported-and-tested-by: syzbot+c584225dabdea2f71...@syzkaller.appspotmail.com
Signed-off-by: Rustam Kovhaev
Link: https://syzkaller.appspot.com/bug?extid=c584225dabdea2f71969
---
fs/ntfs/inode.c | 15 +++
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/fs/ntfs/inode.c b/fs/ntfs/inode.c
index f7e4cbc26eaf..70745aea5106 100644
--- a/fs/ntfs/inode.c
+++ b/fs/ntfs/inode.c
@@ -629,6 +629,13 @@ static int ntfs_read_locked_inode(struct inode *vi)
}
a = ctx->attr;
/* Get the standard information attribute value. */
+ if ((u8 *)a + le16_to_cpu(a->data.resident.value_offset)
+ + le32_to_cpu(
+ a->data.resident.value_length) >
+ (u8 *)ctx->mrec + vol->mft_record_size) {
+ ntfs_error(vi->i_sb, "Corrupt attribute list in inode.");
+ goto unm_err_out;
+ }
si = (STANDARD_INFORMATION*)((u8*)a +
le16_to_cpu(a->data.resident.value_offset));
@@ -731,14 +738,6 @@ static int ntfs_read_locked_inode(struct inode *vi)
goto unm_err_out;
}
} else /* if (!a->non_resident) */ {
- if ((u8*)a + le16_to_cpu(a->data.resident.value_offset)
- + le32_to_cpu(
- a->data.resident.value_length) >
- (u8*)ctx->mrec + vol->mft_record_size) {
- ntfs_error(vi->i_sb, "Corrupt attribute list "
- "in inode.");
- goto unm_err_out;
- }
/* Now copy the attribute list. */
memcpy(ni->attr_list, (u8*)a + le16_to_cpu(
a->data.resident.value_offset),
--
2.30.0
Re: [PATCH] ntfs: move check for valid resident attribute offset and length
On Tue, Feb 16, 2021 at 02:40:37AM +, Anton Altaparmakov wrote: > Hi Rustam, > > Thank you for the patch but it is not quite correct: > > 1) The first delta: yes that is a good idea to add this check but the error > message is incorrect. It should say "Corrupt standard information attribute > in inode." instead. > > 2) The second delta: The check of the attribute list attribute needs to > remain, i.e. your second delta needs to be deleted. > > Please could you address both of the above comments and then resend? Please > then also add: "Acked-by: Anton Altaparmakov " to the patch. > > Thanks a lot in advance! > > Best regards, > > Anton > hi Anton, thank you for the review! I'll resend the patch!
Re: memory leak in bpf
On Mon, Mar 01, 2021 at 09:43:00PM +0100, Dmitry Vyukov wrote: > On Mon, Mar 1, 2021 at 9:39 PM Rustam Kovhaev wrote: > > > > On Mon, Mar 01, 2021 at 08:05:42PM +0100, Dmitry Vyukov wrote: > > > On Mon, Mar 1, 2021 at 5:21 PM Rustam Kovhaev wrote: > > > > > > > > On Wed, Dec 09, 2020 at 10:58:10PM -0800, syzbot wrote: > > > > > syzbot has found a reproducer for the following issue on: > > > > > > > > > > HEAD commit:a68a0262 mm/madvise: remove racy mm ownership check > > > > > git tree: upstream > > > > > console output: > > > > > https://syzkaller.appspot.com/x/log.txt?x=11facf1750 > > > > > kernel config: > > > > > https://syzkaller.appspot.com/x/.config?x=4305fa9ea70c7a9f > > > > > dashboard link: > > > > > https://syzkaller.appspot.com/bug?extid=f3694595248708227d35 > > > > > compiler: gcc (GCC) 10.1.0-syz 20200507 > > > > > syz repro: > > > > > https://syzkaller.appspot.com/x/repro.syz?x=159a961350 > > > > > C reproducer: > > > > > https://syzkaller.appspot.com/x/repro.c?x=11bf712350 > > > > > > > > > > IMPORTANT: if you fix the issue, please add the following tag to the > > > > > commit: > > > > > Reported-by: syzbot+f3694595248708227...@syzkaller.appspotmail.com > > > > > > > > > > Debian GNU/Linux 9 syzkaller ttyS0 > > > > > Warning: Permanently added '10.128.0.9' (ECDSA) to the list of known > > > > > hosts. > > > > > executing program > > > > > executing program > > > > > executing program > > > > > BUG: memory leak > > > > > unreferenced object 0x88810efccc80 (size 64): > > > > > comm "syz-executor334", pid 8460, jiffies 4294945724 (age 13.850s) > > > > > hex dump (first 32 bytes): > > > > > c0 cb 14 04 00 ea ff ff c0 c2 11 04 00 ea ff ff > > > > > c0 56 3f 04 00 ea ff ff 40 18 38 04 00 ea ff ff .V?.@.8. > > > > > backtrace: > > > > > [<36ae98a7>] kmalloc_node include/linux/slab.h:575 > > > > > [inline] > > > > > [<36ae98a7>] bpf_ringbuf_area_alloc > > > > > kernel/bpf/ringbuf.c:94 [inline] > > > > > [<36ae98a7>] bpf_ringbuf_alloc kernel/bpf/ringbuf.c:135 > > > > > [inline] > > > > > [<36ae98a7>] ringbuf_map_alloc kernel/bpf/ringbuf.c:183 > > > > > [inline] > > > > > [<36ae98a7>] ringbuf_map_alloc+0x1be/0x410 > > > > > kernel/bpf/ringbuf.c:150 > > > > > [<d2cb93ae>] find_and_alloc_map kernel/bpf/syscall.c:122 > > > > > [inline] > > > > > [<d2cb93ae>] map_create kernel/bpf/syscall.c:825 [inline] > > > > > [<d2cb93ae>] __do_sys_bpf+0x7d0/0x30a0 > > > > > kernel/bpf/syscall.c:4381 > > > > > [<8feaf393>] do_syscall_64+0x2d/0x70 > > > > > arch/x86/entry/common.c:46 > > > > > [<e1f53cfd>] entry_SYSCALL_64_after_hwframe+0x44/0xa9 > > > > > > > > > > > > > > > > > > i am pretty sure that this one is a false positive > > > > the problem with reproducer is that it does not terminate all of the > > > > child processes that it spawns > > > > > > > > i confirmed that it is a false positive by tracing __fput() and > > > > bpf_map_release(), i ran reproducer, got kmemleak report, then i > > > > manually killed those running leftover processes from reproducer and > > > > then both functions were executed and memory was freed > > > > > > > > i am marking this one as: > > > > #syz invalid > > > > > > Hi Rustam, > > > > > > Thanks for looking into this. > > > > > > I wonder how/where are these objects referenced? If they are not > > > leaked and referenced somewhere, KMEMLEAK should not report them as > > > leaks. > > > So even if this is a false positive for BPF, this is a true positive > > > bug and something to fix for KMEMLEAK ;) > > > And syzbot will probably re-create this bug report soon as this still > > > happens and is not a one-off thi
Re: memory leak in bpf
On Wed, Apr 07, 2021 at 04:35:34PM -0700, Andrii Nakryiko wrote: > On Wed, Apr 7, 2021 at 4:24 PM Rustam Kovhaev wrote: > > > > On Mon, Mar 01, 2021 at 09:43:00PM +0100, Dmitry Vyukov wrote: > > > On Mon, Mar 1, 2021 at 9:39 PM Rustam Kovhaev wrote: > > > > > > > > On Mon, Mar 01, 2021 at 08:05:42PM +0100, Dmitry Vyukov wrote: > > > > > On Mon, Mar 1, 2021 at 5:21 PM Rustam Kovhaev > > > > > wrote: > > > > > > > > > > > > On Wed, Dec 09, 2020 at 10:58:10PM -0800, syzbot wrote: > > > > > > > syzbot has found a reproducer for the following issue on: > > > > > > > > > > > > > > HEAD commit:a68a0262 mm/madvise: remove racy mm ownership > > > > > > > check > > > > > > > git tree: upstream > > > > > > > console output: > > > > > > > https://syzkaller.appspot.com/x/log.txt?x=11facf1750 > > > > > > > kernel config: > > > > > > > https://syzkaller.appspot.com/x/.config?x=4305fa9ea70c7a9f > > > > > > > dashboard link: > > > > > > > https://syzkaller.appspot.com/bug?extid=f3694595248708227d35 > > > > > > > compiler: gcc (GCC) 10.1.0-syz 20200507 > > > > > > > syz repro: > > > > > > > https://syzkaller.appspot.com/x/repro.syz?x=159a961350 > > > > > > > C reproducer: > > > > > > > https://syzkaller.appspot.com/x/repro.c?x=11bf712350 > > > > > > > > > > > > > > IMPORTANT: if you fix the issue, please add the following tag to > > > > > > > the commit: > > > > > > > Reported-by: syzbot+f3694595248708227...@syzkaller.appspotmail.com > > > > > > > > > > > > > > Debian GNU/Linux 9 syzkaller ttyS0 > > > > > > > Warning: Permanently added '10.128.0.9' (ECDSA) to the list of > > > > > > > known hosts. > > > > > > > executing program > > > > > > > executing program > > > > > > > executing program > > > > > > > BUG: memory leak > > > > > > > unreferenced object 0x88810efccc80 (size 64): > > > > > > > comm "syz-executor334", pid 8460, jiffies 4294945724 (age > > > > > > > 13.850s) > > > > > > > hex dump (first 32 bytes): > > > > > > > c0 cb 14 04 00 ea ff ff c0 c2 11 04 00 ea ff ff > > > > > > > > > > > > > > c0 56 3f 04 00 ea ff ff 40 18 38 04 00 ea ff ff > > > > > > > .V?.@.8. > > > > > > > backtrace: > > > > > > > [<36ae98a7>] kmalloc_node include/linux/slab.h:575 > > > > > > > [inline] > > > > > > > [<36ae98a7>] bpf_ringbuf_area_alloc > > > > > > > kernel/bpf/ringbuf.c:94 [inline] > > > > > > > [<36ae98a7>] bpf_ringbuf_alloc > > > > > > > kernel/bpf/ringbuf.c:135 [inline] > > > > > > > [<36ae98a7>] ringbuf_map_alloc > > > > > > > kernel/bpf/ringbuf.c:183 [inline] > > > > > > > [<36ae98a7>] ringbuf_map_alloc+0x1be/0x410 > > > > > > > kernel/bpf/ringbuf.c:150 > > > > > > > [<d2cb93ae>] find_and_alloc_map > > > > > > > kernel/bpf/syscall.c:122 [inline] > > > > > > > [<d2cb93ae>] map_create kernel/bpf/syscall.c:825 > > > > > > > [inline] > > > > > > > [<d2cb93ae>] __do_sys_bpf+0x7d0/0x30a0 > > > > > > > kernel/bpf/syscall.c:4381 > > > > > > > [<8feaf393>] do_syscall_64+0x2d/0x70 > > > > > > > arch/x86/entry/common.c:46 > > > > > > > [<e1f53cfd>] entry_SYSCALL_64_after_hwframe+0x44/0xa9 > > > > > > > > > > > > > > > > > > > > > > > > > > i am pretty sure that this one is a false positive > > > > > > the problem with reproducer is that it does not terminate all of the > > > > > > child processes that it spawns > > > > > > > > >
[PATCH] ntfs: check for valid standard information attribute
we should check for valid STANDARD_INFORMATION attribute offset and
length before trying to access it
Reported-and-tested-by: syzbot+c584225dabdea2f71...@syzkaller.appspotmail.com
Signed-off-by: Rustam Kovhaev
Acked-by: Anton Altaparmakov
Link: https://syzkaller.appspot.com/bug?extid=c584225dabdea2f71969
---
fs/ntfs/inode.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/fs/ntfs/inode.c b/fs/ntfs/inode.c
index f7e4cbc26eaf..be4ff9386ec0 100644
--- a/fs/ntfs/inode.c
+++ b/fs/ntfs/inode.c
@@ -629,6 +629,12 @@ static int ntfs_read_locked_inode(struct inode *vi)
}
a = ctx->attr;
/* Get the standard information attribute value. */
+ if ((u8 *)a + le16_to_cpu(a->data.resident.value_offset)
+ + le32_to_cpu(a->data.resident.value_length) >
+ (u8 *)ctx->mrec + vol->mft_record_size) {
+ ntfs_error(vi->i_sb, "Corrupt standard information attribute in
inode.");
+ goto unm_err_out;
+ }
si = (STANDARD_INFORMATION*)((u8*)a +
le16_to_cpu(a->data.resident.value_offset));
--
2.30.0
Re: KASAN: use-after-free Read in netdevice_event_work_handler
On Thu, Jul 09, 2020 at 04:54:19PM -0700, syzbot wrote: > Hello, > > syzbot found the following crash on: > > HEAD commit:0bddd227 Documentation: update for gcc 4.9 requirement > git tree: upstream > console output: https://syzkaller.appspot.com/x/log.txt?x=1418afb710 > kernel config: https://syzkaller.appspot.com/x/.config?x=66ad203c2bb6d8b > dashboard link: https://syzkaller.appspot.com/bug?extid=20b90969babe05609947 > compiler: gcc (GCC) 10.1.0-syz 20200507 > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12a8edff10 > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=167d3bb710 > > IMPORTANT: if you fix the bug, please add the following tag to the commit: > Reported-by: syzbot+20b90969babe05609...@syzkaller.appspotmail.com > > == > BUG: KASAN: use-after-free in dev_put include/linux/netdevice.h:3853 [inline] > BUG: KASAN: use-after-free in netdevice_event_work_handler+0x15b/0x1b0 > drivers/infiniband/core/roce_gid_mgmt.c:627 > Read of size 8 at addr 88807b13e568 by task kworker/u4:0/7 > > CPU: 0 PID: 7 Comm: kworker/u4:0 Not tainted 5.8.0-rc4-syzkaller #0 > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS > Google 01/01/2011 > Workqueue: gid-cache-wq netdevice_event_work_handler > Call Trace: > __dump_stack lib/dump_stack.c:77 [inline] > dump_stack+0x18f/0x20d lib/dump_stack.c:118 > print_address_description.constprop.0.cold+0xae/0x436 mm/kasan/report.c:383 > __kasan_report mm/kasan/report.c:513 [inline] > kasan_report.cold+0x1f/0x37 mm/kasan/report.c:530 > dev_put include/linux/netdevice.h:3853 [inline] > netdevice_event_work_handler+0x15b/0x1b0 > drivers/infiniband/core/roce_gid_mgmt.c:627 > process_one_work+0x94c/0x1670 kernel/workqueue.c:2269 > worker_thread+0x64c/0x1120 kernel/workqueue.c:2415 > kthread+0x3b5/0x4a0 kernel/kthread.c:291 > ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:293 > > Allocated by task 13061: > save_stack+0x1b/0x40 mm/kasan/common.c:48 > set_track mm/kasan/common.c:56 [inline] > __kasan_kmalloc.constprop.0+0xc2/0xd0 mm/kasan/common.c:494 > kmalloc_node include/linux/slab.h:578 [inline] > kvmalloc_node+0x61/0xf0 mm/util.c:574 > kvmalloc include/linux/mm.h:753 [inline] > kvzalloc include/linux/mm.h:761 [inline] > alloc_netdev_mqs+0x97/0xdc0 net/core/dev.c:9938 > __ip_tunnel_create+0x201/0x580 net/ipv4/ip_tunnel.c:254 > ip_tunnel_init_net+0x32b/0x980 net/ipv4/ip_tunnel.c:1072 > ops_init+0xaf/0x470 net/core/net_namespace.c:151 > setup_net+0x2d8/0x850 net/core/net_namespace.c:341 > copy_net_ns+0x2cf/0x5e0 net/core/net_namespace.c:482 > create_new_namespaces+0x3f6/0xb10 kernel/nsproxy.c:110 > unshare_nsproxy_namespaces+0xbd/0x1f0 kernel/nsproxy.c:231 > ksys_unshare+0x36c/0x9a0 kernel/fork.c:2983 > __do_sys_unshare kernel/fork.c:3051 [inline] > __se_sys_unshare kernel/fork.c:3049 [inline] > __x64_sys_unshare+0x2d/0x40 kernel/fork.c:3049 > do_syscall_64+0x60/0xe0 arch/x86/entry/common.c:384 > entry_SYSCALL_64_after_hwframe+0x44/0xa9 > > Freed by task 13061: > save_stack+0x1b/0x40 mm/kasan/common.c:48 > set_track mm/kasan/common.c:56 [inline] > kasan_set_free_info mm/kasan/common.c:316 [inline] > __kasan_slab_free+0xf5/0x140 mm/kasan/common.c:455 > __cache_free mm/slab.c:3426 [inline] > kfree+0x103/0x2c0 mm/slab.c:3757 > kvfree+0x42/0x50 mm/util.c:603 > device_release+0x71/0x200 drivers/base/core.c:1559 > kobject_cleanup lib/kobject.c:693 [inline] > kobject_release lib/kobject.c:722 [inline] > kref_put include/linux/kref.h:65 [inline] > kobject_put+0x1c0/0x270 lib/kobject.c:739 > put_device+0x1b/0x30 drivers/base/core.c:2779 > free_netdev+0x35d/0x480 net/core/dev.c:10054 > __ip_tunnel_create+0x48f/0x580 net/ipv4/ip_tunnel.c:274 > ip_tunnel_init_net+0x32b/0x980 net/ipv4/ip_tunnel.c:1072 > ops_init+0xaf/0x470 net/core/net_namespace.c:151 > setup_net+0x2d8/0x850 net/core/net_namespace.c:341 > copy_net_ns+0x2cf/0x5e0 net/core/net_namespace.c:482 > create_new_namespaces+0x3f6/0xb10 kernel/nsproxy.c:110 > unshare_nsproxy_namespaces+0xbd/0x1f0 kernel/nsproxy.c:231 > ksys_unshare+0x36c/0x9a0 kernel/fork.c:2983 > __do_sys_unshare kernel/fork.c:3051 [inline] > __se_sys_unshare kernel/fork.c:3049 [inline] > __x64_sys_unshare+0x2d/0x40 kernel/fork.c:3049 > do_syscall_64+0x60/0xe0 arch/x86/entry/common.c:384 > entry_SYSCALL_64_after_hwframe+0x44/0xa9 > > The buggy address belongs to the object at 88807b13e000 > which belongs to the cache kmalloc-4k of size 4096 > The buggy address is located 1384 bytes inside of > 4096-byte region [88807b13e000, 88807b13f000) > The buggy address belongs to the page: > page:ea0001ec4f80 refcount:1 mapcount:0 mapping: > index:0x0 head:ea0001ec4f80 order:1 compound_mapcount:0 > flags: 0xfffe010200(slab|head) > raw: 00fffe010200 ea0001ecce88 ea0001987988 8880aa002000 > raw: 8880
Re: KASAN: use-after-free Read in netdevice_event_work_handler
On Fri, Jul 31, 2020 at 02:11:22PM -0700, Rustam Kovhaev wrote: > On Thu, Jul 09, 2020 at 04:54:19PM -0700, syzbot wrote: > > Hello, > > > > syzbot found the following crash on: > > > > HEAD commit:0bddd227 Documentation: update for gcc 4.9 requirement > > git tree: upstream > > console output: https://syzkaller.appspot.com/x/log.txt?x=1418afb710 > > kernel config: https://syzkaller.appspot.com/x/.config?x=66ad203c2bb6d8b > > dashboard link: https://syzkaller.appspot.com/bug?extid=20b90969babe05609947 > > compiler: gcc (GCC) 10.1.0-syz 20200507 > > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12a8edff10 > > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=167d3bb710 > > > > IMPORTANT: if you fix the bug, please add the following tag to the commit: > > Reported-by: syzbot+20b90969babe05609...@syzkaller.appspotmail.com > > > > == > > BUG: KASAN: use-after-free in dev_put include/linux/netdevice.h:3853 > > [inline] > > BUG: KASAN: use-after-free in netdevice_event_work_handler+0x15b/0x1b0 > > drivers/infiniband/core/roce_gid_mgmt.c:627 > > Read of size 8 at addr 88807b13e568 by task kworker/u4:0/7 > > > > CPU: 0 PID: 7 Comm: kworker/u4:0 Not tainted 5.8.0-rc4-syzkaller #0 > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS > > Google 01/01/2011 > > Workqueue: gid-cache-wq netdevice_event_work_handler > > Call Trace: > > __dump_stack lib/dump_stack.c:77 [inline] > > dump_stack+0x18f/0x20d lib/dump_stack.c:118 > > print_address_description.constprop.0.cold+0xae/0x436 mm/kasan/report.c:383 > > __kasan_report mm/kasan/report.c:513 [inline] > > kasan_report.cold+0x1f/0x37 mm/kasan/report.c:530 > > dev_put include/linux/netdevice.h:3853 [inline] > > netdevice_event_work_handler+0x15b/0x1b0 > > drivers/infiniband/core/roce_gid_mgmt.c:627 > > process_one_work+0x94c/0x1670 kernel/workqueue.c:2269 > > worker_thread+0x64c/0x1120 kernel/workqueue.c:2415 > > kthread+0x3b5/0x4a0 kernel/kthread.c:291 > > ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:293 > > > > Allocated by task 13061: > > save_stack+0x1b/0x40 mm/kasan/common.c:48 > > set_track mm/kasan/common.c:56 [inline] > > __kasan_kmalloc.constprop.0+0xc2/0xd0 mm/kasan/common.c:494 > > kmalloc_node include/linux/slab.h:578 [inline] > > kvmalloc_node+0x61/0xf0 mm/util.c:574 > > kvmalloc include/linux/mm.h:753 [inline] > > kvzalloc include/linux/mm.h:761 [inline] > > alloc_netdev_mqs+0x97/0xdc0 net/core/dev.c:9938 > > __ip_tunnel_create+0x201/0x580 net/ipv4/ip_tunnel.c:254 > > ip_tunnel_init_net+0x32b/0x980 net/ipv4/ip_tunnel.c:1072 > > ops_init+0xaf/0x470 net/core/net_namespace.c:151 > > setup_net+0x2d8/0x850 net/core/net_namespace.c:341 > > copy_net_ns+0x2cf/0x5e0 net/core/net_namespace.c:482 > > create_new_namespaces+0x3f6/0xb10 kernel/nsproxy.c:110 > > unshare_nsproxy_namespaces+0xbd/0x1f0 kernel/nsproxy.c:231 > > ksys_unshare+0x36c/0x9a0 kernel/fork.c:2983 > > __do_sys_unshare kernel/fork.c:3051 [inline] > > __se_sys_unshare kernel/fork.c:3049 [inline] > > __x64_sys_unshare+0x2d/0x40 kernel/fork.c:3049 > > do_syscall_64+0x60/0xe0 arch/x86/entry/common.c:384 > > entry_SYSCALL_64_after_hwframe+0x44/0xa9 > > > > Freed by task 13061: > > save_stack+0x1b/0x40 mm/kasan/common.c:48 > > set_track mm/kasan/common.c:56 [inline] > > kasan_set_free_info mm/kasan/common.c:316 [inline] > > __kasan_slab_free+0xf5/0x140 mm/kasan/common.c:455 > > __cache_free mm/slab.c:3426 [inline] > > kfree+0x103/0x2c0 mm/slab.c:3757 > > kvfree+0x42/0x50 mm/util.c:603 > > device_release+0x71/0x200 drivers/base/core.c:1559 > > kobject_cleanup lib/kobject.c:693 [inline] > > kobject_release lib/kobject.c:722 [inline] > > kref_put include/linux/kref.h:65 [inline] > > kobject_put+0x1c0/0x270 lib/kobject.c:739 > > put_device+0x1b/0x30 drivers/base/core.c:2779 > > free_netdev+0x35d/0x480 net/core/dev.c:10054 > > __ip_tunnel_create+0x48f/0x580 net/ipv4/ip_tunnel.c:274 > > ip_tunnel_init_net+0x32b/0x980 net/ipv4/ip_tunnel.c:1072 > > ops_init+0xaf/0x470 net/core/net_namespace.c:151 > > setup_net+0x2d8/0x850 net/core/net_namespace.c:341 > > copy_net_ns+0x2cf/0x5e0 net/core/net_namespace.c:482 > > create_new_namespaces+0x3f6/0xb10 kernel/nsproxy.c:110 > > unshare_nsproxy_namespaces+0xbd/0x1f0 kernel/nsproxy.c:231 > > ksys_unshare+0x36c/0x9a0 kernel/fork.c:2983 > >
Re: memory leak in do_eventfd
On Thu, Jun 04, 2020 at 09:24:02PM -0700, Eric Biggers wrote: > [+Cc kvm mailing list] > > On Wed, May 20, 2020 at 06:12:17PM -0700, syzbot wrote: > > Hello, > > > > syzbot found the following crash on: > > > > HEAD commit:5a9ffb95 Merge tag '5.7-rc5-smb3-fixes' of git://git.samba.. > > git tree: upstream > > console output: https://syzkaller.appspot.com/x/log.txt?x=10b72a0210 > > kernel config: https://syzkaller.appspot.com/x/.config?x=f8295ae5b3f8268d > > dashboard link: https://syzkaller.appspot.com/bug?extid=f196caa45793d6374707 > > compiler: gcc (GCC) 9.0.0 20181231 (experimental) > > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=17585b7610 > > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=12500a0210 > > > > IMPORTANT: if you fix the bug, please add the following tag to the commit: > > Reported-by: syzbot+f196caa45793d6374...@syzkaller.appspotmail.com > > > > BUG: memory leak > > unreferenced object 0x888117169ac0 (size 64): > > comm "syz-executor012", pid 6609, jiffies 4294942172 (age 13.720s) > > hex dump (first 32 bytes): > > 01 00 00 00 ff ff ff ff 00 00 00 00 00 c9 ff ff > > d0 9a 16 17 81 88 ff ff d0 9a 16 17 81 88 ff ff > > backtrace: > > [<351bb234>] kmalloc include/linux/slab.h:555 [inline] > > [<351bb234>] do_eventfd+0x35/0xf0 fs/eventfd.c:418 > > [] __do_sys_eventfd fs/eventfd.c:443 [inline] > > [ ] __se_sys_eventfd fs/eventfd.c:441 [inline] > > [ ] __x64_sys_eventfd+0x14/0x20 fs/eventfd.c:441 > > [<86d6f989>] do_syscall_64+0x6e/0x220 > > arch/x86/entry/common.c:295 > > [<6c5bcb63>] entry_SYSCALL_64_after_hwframe+0x44/0xa9 > > > > BUG: memory leak > > unreferenced object 0x888117169100 (size 64): > > comm "syz-executor012", pid 6609, jiffies 4294942172 (age 13.720s) > > hex dump (first 32 bytes): > > e8 99 dd 00 00 c9 ff ff e8 99 dd 00 00 c9 ff ff > > 00 00 00 20 00 00 00 00 00 00 00 00 00 00 00 00 ... > > backtrace: > > [<436d2955>] kmalloc include/linux/slab.h:555 [inline] > > [<436d2955>] kzalloc include/linux/slab.h:669 [inline] > > [<436d2955>] kvm_assign_ioeventfd_idx+0x4f/0x270 > > arch/x86/kvm/../../../virt/kvm/eventfd.c:798 > > [ ] kvm_assign_ioeventfd > > arch/x86/kvm/../../../virt/kvm/eventfd.c:934 [inline] > > [ ] kvm_ioeventfd+0xbb/0x194 > > arch/x86/kvm/../../../virt/kvm/eventfd.c:961 > > [ ] kvm_vm_ioctl+0x1e6/0x1030 > > arch/x86/kvm/../../../virt/kvm/kvm_main.c:3670 > > [<5da94937>] vfs_ioctl fs/ioctl.c:47 [inline] > > [<5da94937>] ksys_ioctl+0xa6/0xd0 fs/ioctl.c:771 > > [ ] __do_sys_ioctl fs/ioctl.c:780 [inline] > > [ ] __se_sys_ioctl fs/ioctl.c:778 [inline] > > [ ] __x64_sys_ioctl+0x1a/0x20 fs/ioctl.c:778 > > [<86d6f989>] do_syscall_64+0x6e/0x220 > > arch/x86/entry/common.c:295 > > [<6c5bcb63>] entry_SYSCALL_64_after_hwframe+0x44/0xa9 > > i had to slightly change syzbot reproducer to reproduce this bug in my lab: root@syzkaller:~# diff repro_fixed.c repro_syz.c 371c371 < inject_fault(2); --- > inject_fault(4); the memleak happens when kmalloc() fails in kvm_io_bus_unregister_dev() i also confirmed the leak by running ftrace - eventfd_free_ctx() was not executed when we got to kvm_vm_release() it seems like we should call kvm_iodevice_destructor(), because there could be other devices linked to the bus that's being removed when that particular kmalloc() failure happens i did not want to modify callers of kvm_io_bus_unregister_dev() and i tested the patch below that fixed the leak, but i am not sure if it completely breaks other things in KVM, i would really appreciate if someone can review it: diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 0a68c9d3d3ab..b1996989d576 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -4268,7 +4268,7 @@ int kvm_io_bus_register_dev(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr, void kvm_io_bus_unregister_dev(struct kvm *kvm, enum kvm_bus bus_idx, struct kvm_io_device *dev) { - int i; + int i, j; struct kvm_io_bus *new_bus, *bus; bus = kvm_get_bus(kvm, bus_idx); @@ -4298,6 +4298,13 @@ void kvm_io_bus_unregister_dev(struct kvm *kvm, enum kvm_bus bus_idx, broken: rcu_assign_pointer(kvm->buses[bus_idx], new_bus); synchronize_srcu_expedited(&kvm->srcu); + if (!new_bus) { + for (j = 0; j < bus->dev_count; j++) { + if (j == i) + continue; + kvm_iodevice_destructor(bus->range[j].dev); + } + }
realpath "No such file or directory" warnings when building
tags from KBUILD_OUTPUT directory Reply-To: running 'make ARCH=x86_64 COMPILED_SOURCE=1 cscope tags' in KBUILD_OUTPUT directory produces lots of "No such file or directory" warnings from realpath it seems like commit 4f491bb6ea2a greatly improved tags generation when COMPILED_SOURCE=1 is set, but should we add "-q" flag for realpath in all_compiled_sources() or probably it would be better to fix root cause and make sure that for example we don't try to find objtool sources and exclude other similar dirs during tags generation? what do you think? ... realpath: special.h: No such file or directory realpath: warn.h: No such file or directory realpath: sigchain.c: No such file or directory realpath: sigchain.h: No such file or directory realpath: orc_gen.c: No such file or directory realpath: objtool.c: No such file or directory ...
Re: realpath "No such file or directory" warnings when building tags from KBUILD_OUTPUT directory
On Sun, Aug 09, 2020 at 09:16:27AM +0200, Greg Kroah-Hartman wrote: > On Sat, Aug 08, 2020 at 01:28:22PM -0700, Rustam Kovhaev wrote: > > running 'make ARCH=x86_64 COMPILED_SOURCE=1 cscope tags' in > > KBUILD_OUTPUT directory produces lots of "No such file or directory" > > warnings from realpath > > > > it seems like commit 4f491bb6ea2a greatly improved tags generation when > > COMPILED_SOURCE=1 is set, but should we add "-q" flag for realpath in > > all_compiled_sources() or probably it would be better to fix root cause > > and make sure that for example we don't try to find objtool sources and > > exclude other similar dirs during tags generation? what do you think? > > > > ... > > realpath: special.h: No such file or directory > > realpath: warn.h: No such file or directory > > realpath: sigchain.c: No such file or directory > > realpath: sigchain.h: No such file or directory > > realpath: orc_gen.c: No such file or directory > > realpath: objtool.c: No such file or directory > > ... > > Care to send a patch for this? hi Greg, yes i do, thank you!
[PATCH] scripts/tags.sh: exclude tools directory from tags generation
when COMPILED_SOURCE is set, running 'make ARCH=x86_64 COMPILED_SOURCE=1
cscope tags' in KBUILD_OUTPUT directory produces lots of "No such file
or directory" warnings:
...
realpath: sigchain.h: No such file or directory
realpath: orc_gen.c: No such file or directory
realpath: objtool.c: No such file or directory
...
let's exclude tools directory from tags generation
Fixes: 4f491bb6ea2a ("scripts/tags.sh: collect compiled source precisely")
Link: https://lore.kernel.org/lkml/20200809210056.GA1344537@thinkpad
Signed-off-by: Rustam Kovhaev
---
scripts/tags.sh | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/scripts/tags.sh b/scripts/tags.sh
index 32d3f53af10b..850f4ccb6afc 100755
--- a/scripts/tags.sh
+++ b/scripts/tags.sh
@@ -26,7 +26,11 @@ else
fi
# ignore userspace tools
-ignore="$ignore ( -path ${tree}tools ) -prune -o"
+if [ -n "$COMPILED_SOURCE" ]; then
+ ignore="$ignore ( -path ./tools ) -prune -o"
+else
+ ignore="$ignore ( -path ${tree}tools ) -prune -o"
+fi
# Detect if ALLSOURCE_ARCHS is set. If not, we assume SRCARCH
if [ "${ALLSOURCE_ARCHS}" = "" ]; then
@@ -92,7 +96,7 @@ all_sources()
all_compiled_sources()
{
realpath -es $([ -z "$KBUILD_ABS_SRCTREE" ] && echo --relative-to=.) \
- include/generated/autoconf.h $(find -name "*.cmd" -exec \
+ include/generated/autoconf.h $(find $ignore -name "*.cmd" -exec
\
grep -Poh '(?(?=^source_.* \K).*|(?=^ \K\S).*(?= \\))' {} \+ |
awk '!a[$0]++') | sort -u
}
--
2.28.0
Re: [PATCH] cfg80211: switch from WARN() to pr_warn() in is_user_regdom_saved()
On Wed, Aug 19, 2020 at 10:46:34AM +0200, Johannes Berg wrote: > On Tue, 2020-08-04 at 14:05 -0700, Rustam Kovhaev wrote: > > this warning can be triggered by userspace, so it should not cause a > > panic if panic_on_warn is set > > This is incorrect, it just addresses a particular symptom. I'll make a > proper fix. tyvm!
Re: [PATCH] ntfs: check for valid standard information attribute
On Mon, Feb 22, 2021 at 02:18:50PM +, Anton Altaparmakov wrote: > Rustam would you like to resubmit with an improved/extended description? sure thing, no problem! > when resubmitting with better description, please also add the > "Cc: sta...@vger.kernel.org" line together with the "Signed-off-by", > etc lines (note no need to actually put this in CC: field of the email > iteslf). i will do that, thanks Andrew and Anton
Re: memory leak in bpf
On Wed, Dec 09, 2020 at 10:58:10PM -0800, syzbot wrote: > syzbot has found a reproducer for the following issue on: > > HEAD commit:a68a0262 mm/madvise: remove racy mm ownership check > git tree: upstream > console output: https://syzkaller.appspot.com/x/log.txt?x=11facf1750 > kernel config: https://syzkaller.appspot.com/x/.config?x=4305fa9ea70c7a9f > dashboard link: https://syzkaller.appspot.com/bug?extid=f3694595248708227d35 > compiler: gcc (GCC) 10.1.0-syz 20200507 > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=159a961350 > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=11bf712350 > > IMPORTANT: if you fix the issue, please add the following tag to the commit: > Reported-by: syzbot+f3694595248708227...@syzkaller.appspotmail.com > > Debian GNU/Linux 9 syzkaller ttyS0 > Warning: Permanently added '10.128.0.9' (ECDSA) to the list of known hosts. > executing program > executing program > executing program > BUG: memory leak > unreferenced object 0x88810efccc80 (size 64): > comm "syz-executor334", pid 8460, jiffies 4294945724 (age 13.850s) > hex dump (first 32 bytes): > c0 cb 14 04 00 ea ff ff c0 c2 11 04 00 ea ff ff > c0 56 3f 04 00 ea ff ff 40 18 38 04 00 ea ff ff .V?.@.8. > backtrace: > [<36ae98a7>] kmalloc_node include/linux/slab.h:575 [inline] > [<36ae98a7>] bpf_ringbuf_area_alloc kernel/bpf/ringbuf.c:94 > [inline] > [<36ae98a7>] bpf_ringbuf_alloc kernel/bpf/ringbuf.c:135 [inline] > [<36ae98a7>] ringbuf_map_alloc kernel/bpf/ringbuf.c:183 [inline] > [<36ae98a7>] ringbuf_map_alloc+0x1be/0x410 > kernel/bpf/ringbuf.c:150 > [] find_and_alloc_map kernel/bpf/syscall.c:122 [inline] > [ ] map_create kernel/bpf/syscall.c:825 [inline] > [ ] __do_sys_bpf+0x7d0/0x30a0 kernel/bpf/syscall.c:4381 > [<8feaf393>] do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46 > [ ] entry_SYSCALL_64_after_hwframe+0x44/0xa9 > > i am pretty sure that this one is a false positive the problem with reproducer is that it does not terminate all of the child processes that it spawns i confirmed that it is a false positive by tracing __fput() and bpf_map_release(), i ran reproducer, got kmemleak report, then i manually killed those running leftover processes from reproducer and then both functions were executed and memory was freed i am marking this one as: #syz invalid
Re: memory leak in bpf
On Mon, Mar 01, 2021 at 08:05:42PM +0100, Dmitry Vyukov wrote: > On Mon, Mar 1, 2021 at 5:21 PM Rustam Kovhaev wrote: > > > > On Wed, Dec 09, 2020 at 10:58:10PM -0800, syzbot wrote: > > > syzbot has found a reproducer for the following issue on: > > > > > > HEAD commit:a68a0262 mm/madvise: remove racy mm ownership check > > > git tree: upstream > > > console output: https://syzkaller.appspot.com/x/log.txt?x=11facf1750 > > > kernel config: https://syzkaller.appspot.com/x/.config?x=4305fa9ea70c7a9f > > > dashboard link: > > > https://syzkaller.appspot.com/bug?extid=f3694595248708227d35 > > > compiler: gcc (GCC) 10.1.0-syz 20200507 > > > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=159a961350 > > > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=11bf712350 > > > > > > IMPORTANT: if you fix the issue, please add the following tag to the > > > commit: > > > Reported-by: syzbot+f3694595248708227...@syzkaller.appspotmail.com > > > > > > Debian GNU/Linux 9 syzkaller ttyS0 > > > Warning: Permanently added '10.128.0.9' (ECDSA) to the list of known > > > hosts. > > > executing program > > > executing program > > > executing program > > > BUG: memory leak > > > unreferenced object 0x88810efccc80 (size 64): > > > comm "syz-executor334", pid 8460, jiffies 4294945724 (age 13.850s) > > > hex dump (first 32 bytes): > > > c0 cb 14 04 00 ea ff ff c0 c2 11 04 00 ea ff ff > > > c0 56 3f 04 00 ea ff ff 40 18 38 04 00 ea ff ff .V?.@.8. > > > backtrace: > > > [<36ae98a7>] kmalloc_node include/linux/slab.h:575 [inline] > > > [<36ae98a7>] bpf_ringbuf_area_alloc kernel/bpf/ringbuf.c:94 > > > [inline] > > > [<36ae98a7>] bpf_ringbuf_alloc kernel/bpf/ringbuf.c:135 > > > [inline] > > > [<36ae98a7>] ringbuf_map_alloc kernel/bpf/ringbuf.c:183 > > > [inline] > > > [<36ae98a7>] ringbuf_map_alloc+0x1be/0x410 > > > kernel/bpf/ringbuf.c:150 > > > [<d2cb93ae>] find_and_alloc_map kernel/bpf/syscall.c:122 > > > [inline] > > > [<d2cb93ae>] map_create kernel/bpf/syscall.c:825 [inline] > > > [<d2cb93ae>] __do_sys_bpf+0x7d0/0x30a0 > > > kernel/bpf/syscall.c:4381 > > > [<8feaf393>] do_syscall_64+0x2d/0x70 > > > arch/x86/entry/common.c:46 > > > [<e1f53cfd>] entry_SYSCALL_64_after_hwframe+0x44/0xa9 > > > > > > > > > > i am pretty sure that this one is a false positive > > the problem with reproducer is that it does not terminate all of the > > child processes that it spawns > > > > i confirmed that it is a false positive by tracing __fput() and > > bpf_map_release(), i ran reproducer, got kmemleak report, then i > > manually killed those running leftover processes from reproducer and > > then both functions were executed and memory was freed > > > > i am marking this one as: > > #syz invalid > > Hi Rustam, > > Thanks for looking into this. > > I wonder how/where are these objects referenced? If they are not > leaked and referenced somewhere, KMEMLEAK should not report them as > leaks. > So even if this is a false positive for BPF, this is a true positive > bug and something to fix for KMEMLEAK ;) > And syzbot will probably re-create this bug report soon as this still > happens and is not a one-off thing. hi Dmitry, i haven't thought of it this way, but i guess you are right, it is a kmemleak bug, ideally kmemleak should be aware that there are still running processes holding references to bpf fd/anonymous inodes which in their turn hold references to allocated bpf maps
[PATCH] block: switch to pr_warn() in __device_add_disk()
syzbot triggered a warning while fuzzing with failslab fault injection
enabled
let's convert WARN_ON() to pr_warn()
Reported-and-tested-by: syzbot+f41893bb8c45cd18c...@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?extid=f41893bb8c45cd18cf08
Signed-off-by: Rustam Kovhaev
---
block/genhd.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/block/genhd.c b/block/genhd.c
index 99c64641c314..be9ce35cf0fe 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -822,7 +822,8 @@ static void __device_add_disk(struct device *parent, struct
gendisk *disk,
/* Register BDI before referencing it from bdev */
dev->devt = devt;
ret = bdi_register(bdi, "%u:%u", MAJOR(devt), MINOR(devt));
- WARN_ON(ret);
+ if (ret)
+ pr_warn("%s: failed to register backing dev info\n",
disk->disk_name);
bdi_set_owner(bdi, dev);
blk_register_region(disk_devt(disk), disk->minors, NULL,
exact_match, exact_lock, disk);
--
2.28.0
Re: [PATCH] block: switch to pr_warn() in __device_add_disk()
On Sun, Oct 11, 2020 at 04:53:22PM +0200, Hannes Reinecke wrote:
> On 10/11/20 3:03 PM, Rustam Kovhaev wrote:
> > syzbot triggered a warning while fuzzing with failslab fault injection
> > enabled
> > let's convert WARN_ON() to pr_warn()
> >
> > Reported-and-tested-by:
> > syzbot+f41893bb8c45cd18c...@syzkaller.appspotmail.com
> > Link: https://syzkaller.appspot.com/bug?extid=f41893bb8c45cd18cf08
> > Signed-off-by: Rustam Kovhaev
> > ---
> > block/genhd.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/block/genhd.c b/block/genhd.c
> > index 99c64641c314..be9ce35cf0fe 100644
> > --- a/block/genhd.c
> > +++ b/block/genhd.c
> > @@ -822,7 +822,8 @@ static void __device_add_disk(struct device *parent,
> > struct gendisk *disk,
> > /* Register BDI before referencing it from bdev */
> > dev->devt = devt;
> > ret = bdi_register(bdi, "%u:%u", MAJOR(devt), MINOR(devt));
> > - WARN_ON(ret);
> > + if (ret)
> > + pr_warn("%s: failed to register backing dev info\n",
> > disk->disk_name);
> > bdi_set_owner(bdi, dev);
> > blk_register_region(disk_devt(disk), disk->minors, NULL,
> > exact_match, exact_lock, disk);
> >
> Please, don't. Where is the point in continuing here?
> I'd rather have it fixed up properly, either by having a return value to
> __device_add_disk() or by allowing the caller to check (eg by checking
> GENHD_FL_UP) if the call succeeded.
thank you for the review, it makes sense.
Re: KASAN: use-after-free Read in btrfs_scan_one_device
On Thu, Oct 01, 2020 at 03:35:46PM +0200, David Sterba wrote: > On Thu, Oct 01, 2020 at 03:08:34PM +0200, Dmitry Vyukov wrote: > > On Thu, Oct 1, 2020 at 3:05 PM Dmitry Vyukov wrote: > > > > > > On Wed, Sep 30, 2020 at 8:06 PM David Sterba wrote: > > > > > > > > On Wed, Sep 30, 2020 at 06:57:56PM +0200, David Sterba wrote: > > > > > On Sun, Sep 20, 2020 at 07:12:14AM -0700, syzbot wrote: > > > > > > Hello, > > > > > > > > > > > > syzbot found the following issue on: > > > > > > > > > > > > HEAD commit:eb5f95f1 Merge tag 's390-5.9-6' of > > > > > > git://git.kernel.org/pu.. > > > > > > git tree: upstream > > > > > > console output: > > > > > > https://syzkaller.appspot.com/x/log.txt?x=10a0a8bb90 > > > > > > kernel config: > > > > > > https://syzkaller.appspot.com/x/.config?x=ffe85b197a57c180 > > > > > > dashboard link: > > > > > > https://syzkaller.appspot.com/bug?extid=582e66e5edf36a22c7b0 > > > > > > compiler: gcc (GCC) 10.1.0-syz 20200507 > > > > > > > > > > > > Unfortunately, I don't have any reproducer for this issue yet. > > > > > > > > > > > > IMPORTANT: if you fix the issue, please add the following tag to > > > > > > the commit: > > > > > > Reported-by: syzbot+582e66e5edf36a22c...@syzkaller.appspotmail.com > > > > > > > > > > #syz fix: btrfs: fix overflow when copying corrupt csums for a message > > > > > > > > Johannes spotted that this is not the right fix for this report, I don't > > > > know how to tell syzbot to revert the 'fix:' command, there isn't > > > > 'unfix' (like there's 'undup'). > > > > > > Hi David, > > > > > > I've added "unfix" command: > > > https://github.com/google/syzkaller/pull/2156 > > > > > > Let's give it a try: > > > #syz unfix > > > > > > Thanks > > > > Voilà ! Unfixed: > > https://syzkaller.appspot.com/bug?extid=582e66e5edf36a22c7b0 > > Thanks! the problem is that btrfs_kill_super() frees *fs_info while it is still being referenced by btrfs_scan_one_device() on behalf of another concurrent mount syscall a very simple and dumb fix is to remove that printk that references *fs_info: https://syzkaller.appspot.com/text?tag=Patch&x=123537fb90 but instead, i think proper synchronization is needed here any advice or pointers would be highly appreciated tyvm!
[PATCH v2] KVM: fix memory leak in kvm_io_bus_unregister_dev()
when kmalloc() fails in kvm_io_bus_unregister_dev(), before removing
the bus, we should iterate over all other devices linked to it and call
kvm_iodevice_destructor() for them
Reported-and-tested-by: syzbot+f196caa45793d6374...@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?extid=f196caa45793d6374707
Signed-off-by: Rustam Kovhaev
Reviewed-by: Vitaly Kuznetsov
---
v2:
- remove redundant whitespace
- remove goto statement and use if/else
---
virt/kvm/kvm_main.c | 21 -
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 67cd0b88a6b6..cf88233b819a 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -4332,7 +4332,7 @@ int kvm_io_bus_register_dev(struct kvm *kvm, enum kvm_bus
bus_idx, gpa_t addr,
void kvm_io_bus_unregister_dev(struct kvm *kvm, enum kvm_bus bus_idx,
struct kvm_io_device *dev)
{
- int i;
+ int i, j;
struct kvm_io_bus *new_bus, *bus;
bus = kvm_get_bus(kvm, bus_idx);
@@ -4349,17 +4349,20 @@ void kvm_io_bus_unregister_dev(struct kvm *kvm, enum
kvm_bus bus_idx,
new_bus = kmalloc(struct_size(bus, range, bus->dev_count - 1),
GFP_KERNEL_ACCOUNT);
- if (!new_bus) {
+ if (new_bus) {
+ memcpy(new_bus, bus, sizeof(*bus) + i * sizeof(struct
kvm_io_range));
+ new_bus->dev_count--;
+ memcpy(new_bus->range + i, bus->range + i + 1,
+ (new_bus->dev_count - i) * sizeof(struct kvm_io_range));
+ } else {
pr_err("kvm: failed to shrink bus, removing it completely\n");
- goto broken;
+ for (j = 0; j < bus->dev_count; j++) {
+ if (j == i)
+ continue;
+ kvm_iodevice_destructor(bus->range[j].dev);
+ }
}
- memcpy(new_bus, bus, sizeof(*bus) + i * sizeof(struct kvm_io_range));
- new_bus->dev_count--;
- memcpy(new_bus->range + i, bus->range + i + 1,
- (new_bus->dev_count - i) * sizeof(struct kvm_io_range));
-
-broken:
rcu_assign_pointer(kvm->buses[bus_idx], new_bus);
synchronize_srcu_expedited(&kvm->srcu);
kfree(bus);
--
2.28.0
Re: [PATCH v2] KVM: fix memory leak in kvm_io_bus_unregister_dev()
On Wed, Sep 02, 2020 at 06:34:11PM -0500, Gustavo A. R. Silva wrote:
> Hi,
>
> On 9/2/20 17:57, Rustam Kovhaev wrote:
> > when kmalloc() fails in kvm_io_bus_unregister_dev(), before removing
> > the bus, we should iterate over all other devices linked to it and call
> > kvm_iodevice_destructor() for them
> >
> > Reported-and-tested-by:
> > syzbot+f196caa45793d6374...@syzkaller.appspotmail.com
> > Link: https://syzkaller.appspot.com/bug?extid=f196caa45793d6374707
> > Signed-off-by: Rustam Kovhaev
> > Reviewed-by: Vitaly Kuznetsov
>
> I think it's worthwhile to add a Fixes tag for this, too.
>
> Please, see more comments below...
>
> > ---
> > v2:
> > - remove redundant whitespace
> > - remove goto statement and use if/else
> > ---
> > virt/kvm/kvm_main.c | 21 -
> > 1 file changed, 12 insertions(+), 9 deletions(-)
> >
> > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> > index 67cd0b88a6b6..cf88233b819a 100644
> > --- a/virt/kvm/kvm_main.c
> > +++ b/virt/kvm/kvm_main.c
> > @@ -4332,7 +4332,7 @@ int kvm_io_bus_register_dev(struct kvm *kvm, enum
> > kvm_bus bus_idx, gpa_t addr,
> > void kvm_io_bus_unregister_dev(struct kvm *kvm, enum kvm_bus bus_idx,
> >struct kvm_io_device *dev)
> > {
> > - int i;
> > + int i, j;
> > struct kvm_io_bus *new_bus, *bus;
> >
> > bus = kvm_get_bus(kvm, bus_idx);
> > @@ -4349,17 +4349,20 @@ void kvm_io_bus_unregister_dev(struct kvm *kvm,
> > enum kvm_bus bus_idx,
> >
> > new_bus = kmalloc(struct_size(bus, range, bus->dev_count - 1),
> > GFP_KERNEL_ACCOUNT);
> > - if (!new_bus) {
> > + if (new_bus) {
> > + memcpy(new_bus, bus, sizeof(*bus) + i * sizeof(struct
> > kvm_io_range));
>
> ^^^
> It seems that you can use struct_size() here (see the allocation code
> above)...
>
> > + new_bus->dev_count--;
> > + memcpy(new_bus->range + i, bus->range + i + 1,
> > + (new_bus->dev_count - i) * sizeof(struct kvm_io_range));
>
> ^^^
> ...and, if possible, you can also use flex_array_size() here.
>
> Thanks
> --
> Gustavo
>
> > + } else {
> > pr_err("kvm: failed to shrink bus, removing it completely\n");
> > - goto broken;
> > + for (j = 0; j < bus->dev_count; j++) {
> > + if (j == i)
> > + continue;
> > + kvm_iodevice_destructor(bus->range[j].dev);
> > + }
> > }
> >
> > - memcpy(new_bus, bus, sizeof(*bus) + i * sizeof(struct kvm_io_range));
> > - new_bus->dev_count--;
> > - memcpy(new_bus->range + i, bus->range + i + 1,
> > - (new_bus->dev_count - i) * sizeof(struct kvm_io_range));
> > -
> > -broken:
> > rcu_assign_pointer(kvm->buses[bus_idx], new_bus);
> > synchronize_srcu_expedited(&kvm->srcu);
> > kfree(bus);
> >
hi Gustavo, thank you for the review, i'll send the new patch.
Vitaly, i think i will need to drop your "Reviewed-by", because there is
going to be a bit more changes
[PATCH] ntfs: add check for mft record size in superblock
number of bytes allocated for mft record should be equal to the mft
record size stored in ntfs superblock
as reported by syzbot, userspace might trigger out-of-bounds read by
dereferencing ctx->attr in ntfs_attr_find()
Reported-and-tested-by: syzbot+aed06913f36eff9b5...@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?extid=aed06913f36eff9b544e
Signed-off-by: Rustam Kovhaev
---
fs/ntfs/inode.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/fs/ntfs/inode.c b/fs/ntfs/inode.c
index 9bb9f0952b18..6407af7c2e4f 100644
--- a/fs/ntfs/inode.c
+++ b/fs/ntfs/inode.c
@@ -1810,6 +1810,12 @@ int ntfs_read_inode_mount(struct inode *vi)
brelse(bh);
}
+ if (m->bytes_allocated != vol->mft_record_size) {
+ ntfs_error(sb, "Incorrect mft record size [%u] in superblock,
should be [%u].",
+ m->bytes_allocated, vol->mft_record_size);
+ goto err_out;
+ }
+
/* Apply the mst fixups. */
if (post_read_mst_fixup((NTFS_RECORD*)m, vol->mft_record_size)) {
/* FIXME: Try to use the $MFTMirr now. */
--
2.28.0
Re: [PATCH] ntfs: add check for mft record size in superblock
On Mon, Aug 24, 2020 at 01:44:06AM +, Anton Altaparmakov wrote: > Hi Rustam, > > Thank you for the patch but it introduces an endianness bug - you have to us > le32_to_cpu(m->bytes_allocated) both when doing the comparison and then > printing the message. > > Also, please drop the square brackets. Wherever the driver prints such > things it never uses brackets around the numbers and it would be better to > have this consistent throughout. > > Can you please resend with the above issues addressed? You can then also add > to the commit message: > > Acked-by: Anton Altaparmakov > > Thanks! > > Best regards, > > Anton > hi Anton, thank you for the review, my bad, i'll get it fixed and i'll resend the patch
[PATCH] ntfs: add check for mft record size in superblock
number of bytes allocated for mft record should be equal to the mft
record size stored in ntfs superblock
as reported by syzbot, userspace might trigger out-of-bounds read by
dereferencing ctx->attr in ntfs_attr_find()
Reported-and-tested-by: syzbot+aed06913f36eff9b5...@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?extid=aed06913f36eff9b544e
Signed-off-by: Rustam Kovhaev
Acked-by: Anton Altaparmakov
---
fs/ntfs/inode.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/fs/ntfs/inode.c b/fs/ntfs/inode.c
index 9bb9f0952b18..caf563981532 100644
--- a/fs/ntfs/inode.c
+++ b/fs/ntfs/inode.c
@@ -1810,6 +1810,12 @@ int ntfs_read_inode_mount(struct inode *vi)
brelse(bh);
}
+ if (le32_to_cpu(m->bytes_allocated) != vol->mft_record_size) {
+ ntfs_error(sb, "Incorrect mft record size %u in superblock,
should be %u.",
+ le32_to_cpu(m->bytes_allocated),
vol->mft_record_size);
+ goto err_out;
+ }
+
/* Apply the mst fixups. */
if (post_read_mst_fixup((NTFS_RECORD*)m, vol->mft_record_size)) {
/* FIXME: Try to use the $MFTMirr now. */
--
2.28.0
Re: [PATCH] KVM: fix memory leak in kvm_io_bus_unregister_dev()
On Tue, Sep 01, 2020 at 06:25:42PM +0200, Vitaly Kuznetsov wrote:
> Rustam Kovhaev writes:
>
> > when kmalloc() fails in kvm_io_bus_unregister_dev(), before removing
> > the bus, we should iterate over all other devices linked to it and call
> > kvm_iodevice_destructor() for them
> >
> > Reported-and-tested-by:
> > syzbot+f196caa45793d6374...@syzkaller.appspotmail.com
> > Link: https://syzkaller.appspot.com/bug?extid=f196caa45793d6374707
> > Signed-off-by: Rustam Kovhaev
> > ---
> > virt/kvm/kvm_main.c | 7 ++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> > index 67cd0b88a6b6..646aa7b82548 100644
> > --- a/virt/kvm/kvm_main.c
> > +++ b/virt/kvm/kvm_main.c
> > @@ -4332,7 +4332,7 @@ int kvm_io_bus_register_dev(struct kvm *kvm, enum
> > kvm_bus bus_idx, gpa_t addr,
> > void kvm_io_bus_unregister_dev(struct kvm *kvm, enum kvm_bus bus_idx,
> >struct kvm_io_device *dev)
> > {
> > - int i;
> > + int i, j;
> > struct kvm_io_bus *new_bus, *bus;
> >
> > bus = kvm_get_bus(kvm, bus_idx);
> > @@ -4351,6 +4351,11 @@ void kvm_io_bus_unregister_dev(struct kvm *kvm, enum
> > kvm_bus bus_idx,
> > GFP_KERNEL_ACCOUNT);
> > if (!new_bus) {
> ^^^ redundant space
>
> > pr_err("kvm: failed to shrink bus, removing it completely\n");
> > + for (j = 0; j < bus->dev_count; j++) {
> > + if (j == i)
> > + continue;
> > + kvm_iodevice_destructor(bus->range[j].dev);
> > + }
> > goto broken;
>
> The name of the label is really misleading (as it is not actually a
> failure path), I'd even suggest we get rid of this goto completely,
> something like
>
> new_bus = kmalloc(struct_size(bus, range, bus->dev_count - 1),
> GFP_KERNEL_ACCOUNT);
> if (new_bus) {
> memcpy(new_bus, bus, sizeof(*bus) + i * sizeof(struct
> kvm_io_range));
> new_bus->dev_count--;
> memcpy(new_bus->range + i, bus->range + i + 1,
> (new_bus->dev_count - i) * sizeof(struct kvm_io_range));
> } else {
> pr_err("kvm: failed to shrink bus, removing it completely\n");
> for (j = 0; j < bus->dev_count; j++) {
> if (j == i)
> continue;
> kvm_iodevice_destructor(bus->range[j].dev);
> }
>
> rcu_assign_pointer(kvm->buses[bus_idx], new_bus);
> synchronize_srcu_expedited(&kvm->srcu);
> kfree(bus);
> return;
>
>
> > }
>
> None of the above should block the fix IMO, so:
>
> Reviewed-by: Vitaly Kuznetsov
>
> --
> Vitaly
>
hi Vitaly, thank you for the review! i'll send the new patch
Re: [PATCH] veth: fix memory leak in veth_newlink()
On Tue, Sep 01, 2020 at 01:01:27PM -0700, David Miller wrote: > From: Rustam Kovhaev > Date: Sun, 30 Aug 2020 06:13:36 -0700 > > > when register_netdevice(dev) fails we should check whether struct > > veth_rq has been allocated via ndo_init callback and free it, because, > > depending on the code path, register_netdevice() might not call > > priv_destructor() callback > > > > Reported-and-tested-by: > > syzbot+59ef240dd8f0ed759...@syzkaller.appspotmail.com > > Link: https://syzkaller.appspot.com/bug?extid=59ef240dd8f0ed7598a8 > > Signed-off-by: Rustam Kovhaev > > I think I agree with Toshiaki here. There is no reason why the > rollback_registered() path of register_netdevice() should behave > differently from the normal control flow. > > Any code path that invokes ->ndo_uninit() should probably also > invoke the priv destructor. hi David, thank you for the review! > > The question is why does the err_uninit: label of register_netdevice > behave differently from rollback_registered()? If there is a reason, > it should be documented in a comment or similar. If it is wrong, > it should be corrected. good question, that i do not know, i'll review it
[PATCH] KVM: fix memory leak in kvm_io_bus_unregister_dev()
when kmalloc() fails in kvm_io_bus_unregister_dev(), before removing
the bus, we should iterate over all other devices linked to it and call
kvm_iodevice_destructor() for them
Reported-and-tested-by: syzbot+f196caa45793d6374...@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?extid=f196caa45793d6374707
Signed-off-by: Rustam Kovhaev
---
virt/kvm/kvm_main.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 67cd0b88a6b6..646aa7b82548 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -4332,7 +4332,7 @@ int kvm_io_bus_register_dev(struct kvm *kvm, enum kvm_bus
bus_idx, gpa_t addr,
void kvm_io_bus_unregister_dev(struct kvm *kvm, enum kvm_bus bus_idx,
struct kvm_io_device *dev)
{
- int i;
+ int i, j;
struct kvm_io_bus *new_bus, *bus;
bus = kvm_get_bus(kvm, bus_idx);
@@ -4351,6 +4351,11 @@ void kvm_io_bus_unregister_dev(struct kvm *kvm, enum
kvm_bus bus_idx,
GFP_KERNEL_ACCOUNT);
if (!new_bus) {
pr_err("kvm: failed to shrink bus, removing it completely\n");
+ for (j = 0; j < bus->dev_count; j++) {
+ if (j == i)
+ continue;
+ kvm_iodevice_destructor(bus->range[j].dev);
+ }
goto broken;
}
--
2.28.0
[PATCH] veth: fix memory leak in veth_newlink()
when register_netdevice(dev) fails we should check whether struct veth_rq has been allocated via ndo_init callback and free it, because, depending on the code path, register_netdevice() might not call priv_destructor() callback Reported-and-tested-by: syzbot+59ef240dd8f0ed759...@syzkaller.appspotmail.com Link: https://syzkaller.appspot.com/bug?extid=59ef240dd8f0ed7598a8 Signed-off-by: Rustam Kovhaev --- drivers/net/veth.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/net/veth.c b/drivers/net/veth.c index a475f48d43c4..e40ca62a046a 100644 --- a/drivers/net/veth.c +++ b/drivers/net/veth.c @@ -1394,7 +1394,9 @@ static int veth_newlink(struct net *src_net, struct net_device *dev, return 0; err_register_dev: - /* nothing to do */ + priv = netdev_priv(dev); + if (priv->rq) + veth_dev_free(dev); err_configure_peer: unregister_netdevice(peer); return err; -- 2.28.0
Re: [PATCH] veth: fix memory leak in veth_newlink()
On Mon, Aug 31, 2020 at 09:16:32AM +0900, Toshiaki Makita wrote: > On 2020/08/30 22:13, Rustam Kovhaev wrote: > > when register_netdevice(dev) fails we should check whether struct > > veth_rq has been allocated via ndo_init callback and free it, because, > > depending on the code path, register_netdevice() might not call > > priv_destructor() callback > > AFAICS, register_netdevice() always goto err_uninit and calls > priv_destructor() > on failure after ndo_init() succeeded. > So I could not find such a code path. > Would you elaborate on it? in net/core/dev.c:9863, where register_netdevice() calls rollback_registered(), which does not call priv_destructor(), then register_netdevice() returns error net/core/dev.c:9884
[PATCH] staging: wlan-ng: fix out of bounds read in prism2sta_probe_usb()
let's use usb_find_common_endpoints() to discover endpoints, it does all
necessary checks for type and xfer direction
remove memset() in hfa384x_create(), because we now assign endpoints in
prism2sta_probe_usb() and because create_wlan() uses kzalloc() to
allocate hfa384x struct before calling hfa384x_create()
Fixes: faaff9765664 ("staging: wlan-ng: properly check endpoint types")
Reported-and-tested-by: syzbot+22794221ab96b0bab...@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?extid=22794221ab96b0bab53a
Signed-off-by: Rustam Kovhaev
---
drivers/staging/wlan-ng/hfa384x_usb.c | 5 -
drivers/staging/wlan-ng/prism2usb.c | 19 ++-
2 files changed, 6 insertions(+), 18 deletions(-)
diff --git a/drivers/staging/wlan-ng/hfa384x_usb.c
b/drivers/staging/wlan-ng/hfa384x_usb.c
index fa1bf8b069fd..2720f7319a3d 100644
--- a/drivers/staging/wlan-ng/hfa384x_usb.c
+++ b/drivers/staging/wlan-ng/hfa384x_usb.c
@@ -524,13 +524,8 @@ static void hfa384x_usb_defer(struct work_struct *data)
*/
void hfa384x_create(struct hfa384x *hw, struct usb_device *usb)
{
- memset(hw, 0, sizeof(*hw));
hw->usb = usb;
- /* set up the endpoints */
- hw->endp_in = usb_rcvbulkpipe(usb, 1);
- hw->endp_out = usb_sndbulkpipe(usb, 2);
-
/* Set up the waitq */
init_waitqueue_head(&hw->cmdq);
diff --git a/drivers/staging/wlan-ng/prism2usb.c
b/drivers/staging/wlan-ng/prism2usb.c
index 456603fd26c0..4b08dc1da4f9 100644
--- a/drivers/staging/wlan-ng/prism2usb.c
+++ b/drivers/staging/wlan-ng/prism2usb.c
@@ -61,23 +61,14 @@ static int prism2sta_probe_usb(struct usb_interface
*interface,
const struct usb_device_id *id)
{
struct usb_device *dev;
- const struct usb_endpoint_descriptor *epd;
- const struct usb_host_interface *iface_desc = interface->cur_altsetting;
+ struct usb_endpoint_descriptor *bulk_in, *bulk_out;
+ struct usb_host_interface *iface_desc = interface->cur_altsetting;
struct wlandevice *wlandev = NULL;
struct hfa384x *hw = NULL;
int result = 0;
- if (iface_desc->desc.bNumEndpoints != 2) {
- result = -ENODEV;
- goto failed;
- }
-
- result = -EINVAL;
- epd = &iface_desc->endpoint[1].desc;
- if (!usb_endpoint_is_bulk_in(epd))
- goto failed;
- epd = &iface_desc->endpoint[2].desc;
- if (!usb_endpoint_is_bulk_out(epd))
+ result = usb_find_common_endpoints(iface_desc, &bulk_in, &bulk_out,
NULL, NULL);
+ if (result)
goto failed;
dev = interface_to_usbdev(interface);
@@ -96,6 +87,8 @@ static int prism2sta_probe_usb(struct usb_interface
*interface,
}
/* Initialize the hw data */
+ hw->endp_in = usb_rcvbulkpipe(dev, bulk_in->bEndpointAddress);
+ hw->endp_out = usb_sndbulkpipe(dev, bulk_out->bEndpointAddress);
hfa384x_create(hw, dev);
hw->wlandev = wlandev;
--
2.27.0
Re: KASAN: use-after-free Read in netdevice_event_work_handler
On Sun, Aug 02, 2020 at 07:22:26PM -0300, Jason Gunthorpe wrote:
> On Fri, Jul 31, 2020 at 02:11:22PM -0700, Rustam Kovhaev wrote:
>
> > IB roce driver receives NETDEV_UNREGISTER event, calls dev_hold() and
> > schedules work item to execute, and before wq gets a chance to complete
> > it, we return to ip_tunnel.c:274 and call free_netdev(), and then later
> > we get UAF when scheduled function references already freed net_device
> >
> > i added verbose logging to ip_tunnel.c to see pcpu_refcnt:
> > + pr_info("about to free_netdev(dev) dev->pcpu_refcnt %d",
> > netdev_refcnt_read(dev));
> >
> > and got the following:
> > [ 410.220127][ T2944] ip_tunnel: about to free_netdev(dev)
> > dev->pcpu_refcnt 8
>
> I think there is a missing call to netdev_wait_allrefs() in the
> rollback_registered_many().
calling it there leads to rtnl deadlock, i think we should call
net_set_todo(), so that later when we call rtnl_unlock() it will
execute netdev_run_todo() and there it will proceed to calling
netdev_wait_allrefs(), but in ip tunnel i will need get
free_netdev() to be called after we unlock rtnl mutex
i'll try to send a new patch for review
> The normal success flow has this wait after delivering
> NETDEV_UNREGISTER, the error unwind for register_netdevice should as
> well.
>
> If the netdevice can progress to free while a dev_hold is active I
> think it means dev_hold is functionally useless.
good point
[PATCH] cfg80211: switch from WARN() to pr_warn() in is_user_regdom_saved()
this warning can be triggered by userspace, so it should not cause a
panic if panic_on_warn is set
Reported-and-tested-by: syzbot+d451401ffd00a6067...@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?extid=d451401ffd00a60677ee
Signed-off-by: Rustam Kovhaev
---
net/wireless/reg.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 0d74a31ef0ab..86355938ae8f 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -415,10 +415,10 @@ static bool is_user_regdom_saved(void)
return false;
/* This would indicate a mistake on the design */
- if (WARN(!is_world_regdom(user_alpha2) && !is_an_alpha2(user_alpha2),
-"Unexpected user alpha2: %c%c\n",
-user_alpha2[0], user_alpha2[1]))
+ if (!is_world_regdom(user_alpha2) && !is_an_alpha2(user_alpha2)) {
+ pr_warn("Unexpected user_alpha2: %c%c\n", user_alpha2[0],
user_alpha2[1]);
return false;
+ }
return true;
}
--
2.27.0
[RESEND PATCH v2] KVM: fix memory leak in kvm_io_bus_unregister_dev()
when kmalloc() fails in kvm_io_bus_unregister_dev(), before removing
the bus, we should iterate over all other devices linked to it and call
kvm_iodevice_destructor() for them
Fixes: 90db10434b16 ("KVM: kvm_io_bus_unregister_dev() should never fail")
Cc: sta...@vger.kernel.org
Reported-and-tested-by: syzbot+f196caa45793d6374...@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?extid=f196caa45793d6374707
Signed-off-by: Rustam Kovhaev
Reviewed-by: Vitaly Kuznetsov
---
v2:
- remove redundant whitespace
- remove goto statement and use if/else
- add Fixes tag
---
virt/kvm/kvm_main.c | 21 -
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 67cd0b88a6b6..cf88233b819a 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -4332,7 +4332,7 @@ int kvm_io_bus_register_dev(struct kvm *kvm, enum kvm_bus
bus_idx, gpa_t addr,
void kvm_io_bus_unregister_dev(struct kvm *kvm, enum kvm_bus bus_idx,
struct kvm_io_device *dev)
{
- int i;
+ int i, j;
struct kvm_io_bus *new_bus, *bus;
bus = kvm_get_bus(kvm, bus_idx);
@@ -4349,17 +4349,20 @@ void kvm_io_bus_unregister_dev(struct kvm *kvm, enum
kvm_bus bus_idx,
new_bus = kmalloc(struct_size(bus, range, bus->dev_count - 1),
GFP_KERNEL_ACCOUNT);
- if (!new_bus) {
+ if (new_bus) {
+ memcpy(new_bus, bus, sizeof(*bus) + i * sizeof(struct
kvm_io_range));
+ new_bus->dev_count--;
+ memcpy(new_bus->range + i, bus->range + i + 1,
+ (new_bus->dev_count - i) * sizeof(struct kvm_io_range));
+ } else {
pr_err("kvm: failed to shrink bus, removing it completely\n");
- goto broken;
+ for (j = 0; j < bus->dev_count; j++) {
+ if (j == i)
+ continue;
+ kvm_iodevice_destructor(bus->range[j].dev);
+ }
}
- memcpy(new_bus, bus, sizeof(*bus) + i * sizeof(struct kvm_io_range));
- new_bus->dev_count--;
- memcpy(new_bus->range + i, bus->range + i + 1,
- (new_bus->dev_count - i) * sizeof(struct kvm_io_range));
-
-broken:
rcu_assign_pointer(kvm->buses[bus_idx], new_bus);
synchronize_srcu_expedited(&kvm->srcu);
kfree(bus);
--
2.28.0
[PATCH] reiserfs: add check for an invalid ih_entry_count
when directory item has an invalid value set for ih_entry_count it might
trigger use-after-free or out-of-bounds read in bin_search_in_dir_item()
ih_entry_count * IH_SIZE for directory item should not be larger than
ih_item_len
Reported-and-tested-by: syzbot+83b6f7cf9922cae5c...@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?extid=83b6f7cf9922cae5c4d7
Signed-off-by: Rustam Kovhaev
---
fs/reiserfs/stree.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/fs/reiserfs/stree.c b/fs/reiserfs/stree.c
index 8bf88d690729..476a7ff49482 100644
--- a/fs/reiserfs/stree.c
+++ b/fs/reiserfs/stree.c
@@ -454,6 +454,12 @@ static int is_leaf(char *buf, int blocksize, struct
buffer_head *bh)
"(second one): %h", ih);
return 0;
}
+ if (is_direntry_le_ih(ih) && (ih_item_len(ih) <
(ih_entry_count(ih) * IH_SIZE))) {
+ reiserfs_warning(NULL, "reiserfs-5093",
+"item entry count seems wrong %h",
+ih);
+ return 0;
+ }
prev_location = ih_location(ih);
}
--
2.28.0
Re: KASAN: use-after-free Read in v4l2_fh_init
On Fri, Apr 19, 2019 at 07:36:05AM -0700, syzbot wrote: > Hello, > > syzbot found the following crash on: > > HEAD commit:d34f9519 usb-fuzzer: main usb gadget fuzzer driver > git tree: https://github.com/google/kasan/tree/usb-fuzzer > console output: https://syzkaller.appspot.com/x/log.txt?x=125bbb5b20 > kernel config: https://syzkaller.appspot.com/x/.config?x=c73d1bb5aeaeae20 > dashboard link: https://syzkaller.appspot.com/bug?extid=c025d34b8eaa54c571b8 > compiler: gcc (GCC) 9.0.0 20181231 (experimental) > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1513ac1d20 > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=13555c1d20 > > IMPORTANT: if you fix the bug, please add the following tag to the commit: > Reported-by: syzbot+c025d34b8eaa54c57...@syzkaller.appspotmail.com > > em28xx 1-1:5.176: failed to create media graph > em28xx 1-1:5.176: V4L2 device video32 deregistered > em28xx 1-1:5.176: Binding DVB extension > == > em28xx 1-1:5.176: no endpoint for DVB mode and transfer type 0 > BUG: KASAN: use-after-free in v4l2_fh_init+0x24c/0x290 > drivers/media/v4l2-core/v4l2-fh.c:33 > Read of size 8 at addr 8880a4f149d0 by task v4l_id/5313 > > CPU: 1 PID: 5313 Comm: v4l_id Not tainted 5.1.0-rc5-319617-gd34f951 #4 > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS > Google 01/01/2011 > em28xx 1-1:5.176: failed to pre-allocate USB transfer buffers for DVB. > Call Trace: > __dump_stack lib/dump_stack.c:77 [inline] > dump_stack+0xe8/0x16e lib/dump_stack.c:113 > print_address_description+0x6c/0x236 mm/kasan/report.c:187 > em28xx 1-1:5.176: Registering input extension > kasan_report.cold+0x1a/0x3c mm/kasan/report.c:317 > v4l2_fh_init+0x24c/0x290 drivers/media/v4l2-core/v4l2-fh.c:33 > v4l2_fh_open+0x8d/0xd0 drivers/media/v4l2-core/v4l2-fh.c:71 > em28xx_v4l2_open+0x11f/0x470 drivers/media/usb/em28xx/em28xx-video.c:2184 > v4l2_open+0x1b6/0x360 drivers/media/v4l2-core/v4l2-dev.c:427 > chrdev_open+0x220/0x5d0 fs/char_dev.c:417 > do_dentry_open+0x49c/0x1130 fs/open.c:777 > do_last fs/namei.c:3416 [inline] > path_openat+0x147d/0x40b0 fs/namei.c:3533 > do_filp_open+0x1a6/0x280 fs/namei.c:3563 > do_sys_open+0x3c5/0x590 fs/open.c:1069 > do_syscall_64+0xcf/0x4f0 arch/x86/entry/common.c:290 > entry_SYSCALL_64_after_hwframe+0x49/0xbe > RIP: 0033:0x7fdb23b8d120 > Code: 48 8b 15 1b 4d 2b 00 f7 d8 64 89 02 83 c8 ff c3 90 90 90 90 90 90 90 > 90 90 90 83 3d d5 a4 2b 00 00 75 10 b8 02 00 00 00 0f 05 <48> 3d 01 f0 ff ff > 73 31 c3 48 83 ec 08 e8 5e 8c 01 00 48 89 04 24 > RSP: 002b:7ffc1aebde18 EFLAGS: 0246 ORIG_RAX: 0002 > RAX: ffda RBX: 7ffc1aebdf78 RCX: 7fdb23b8d120 > RDX: 7fdb23e42138 RSI: RDI: 7ffc1aebef17 > RBP: R08: R09: > R10: R11: 0246 R12: 00400884 > R13: 7ffc1aebdf70 R14: R15: > > Allocated by task 12: > set_track mm/kasan/common.c:87 [inline] > __kasan_kmalloc mm/kasan/common.c:497 [inline] > __kasan_kmalloc.constprop.0+0xbf/0xd0 mm/kasan/common.c:470 > kmalloc include/linux/slab.h:547 [inline] > kzalloc include/linux/slab.h:742 [inline] > em28xx_v4l2_init drivers/media/usb/em28xx/em28xx-video.c:2563 [inline] > em28xx_v4l2_init.cold+0x93/0x3112 > drivers/media/usb/em28xx/em28xx-video.c:2541 > em28xx_init_extension+0x13a/0x200 > drivers/media/usb/em28xx/em28xx-core.c:1128 > request_module_async+0x62/0x70 drivers/media/usb/em28xx/em28xx-cards.c:3300 > process_one_work+0x90f/0x1580 kernel/workqueue.c:2269 > worker_thread+0x9b/0xe20 kernel/workqueue.c:2415 > kthread+0x313/0x420 kernel/kthread.c:253 > ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:352 > > Freed by task 12: > set_track mm/kasan/common.c:87 [inline] > __kasan_slab_free+0x130/0x180 mm/kasan/common.c:459 > slab_free_hook mm/slub.c:1429 [inline] > slab_free_freelist_hook+0x5e/0x140 mm/slub.c:1456 > slab_free mm/slub.c:3003 [inline] > kfree+0xce/0x280 mm/slub.c:3958 > em28xx_free_v4l2 drivers/media/usb/em28xx/em28xx-video.c:2149 [inline] > kref_put include/linux/kref.h:67 [inline] > em28xx_v4l2_init drivers/media/usb/em28xx/em28xx-video.c:2920 [inline] > em28xx_v4l2_init.cold+0x2cf/0x3112 > drivers/media/usb/em28xx/em28xx-video.c:2541 > em28xx_init_extension+0x13a/0x200 > drivers/media/usb/em28xx/em28xx-core.c:1128 > request_module_async+0x62/0x70 drivers/media/usb/em28xx/em28xx-cards.c:3300 > process_one_work+0x90f/0x1580 kernel/workqueue.c:2269 > worker_thread+0x9b/0xe20 kernel/workqueue.c:2415 > kthread+0x313/0x420 kernel/kthread.c:253 > ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:352 > > The buggy address belongs to the object at 8880a4f14200 > which belongs to the cache kmalloc-8k of size 8192 > The buggy address is located 2000 bytes inside of > 8192-byte region [8880a
[PATCH] staging: wlan-ng: properly check endpoint types
As syzkaller detected, wlan-ng driver submits bulk urb without checking
that the endpoint type is actually bulk, add usb_urb_ep_type_check()
Reported-and-tested-by: syzbot+c2a1fa67c02faa0de...@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?extid=c2a1fa67c02faa0de723
Signed-off-by: Rustam Kovhaev
---
drivers/staging/wlan-ng/hfa384x_usb.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/staging/wlan-ng/hfa384x_usb.c
b/drivers/staging/wlan-ng/hfa384x_usb.c
index fa1bf8b069fd..7cde60ea68a2 100644
--- a/drivers/staging/wlan-ng/hfa384x_usb.c
+++ b/drivers/staging/wlan-ng/hfa384x_usb.c
@@ -339,6 +339,12 @@ static int submit_rx_urb(struct hfa384x *hw, gfp_t
memflags)
hw->rx_urb_skb = skb;
+ result = usb_urb_ep_type_check(&hw->rx_urb);
+ if (result) {
+ netdev_warn(hw->wlandev->netdev, "invalid rx endpoint");
+ goto cleanup;
+ }
+
result = -ENOLINK;
if (!hw->wlandev->hwremoved &&
!test_bit(WORK_RX_HALT, &hw->usb_flags)) {
@@ -354,6 +360,7 @@ static int submit_rx_urb(struct hfa384x *hw, gfp_t memflags)
}
}
+cleanup:
/* Don't leak memory if anything should go wrong */
if (result != 0) {
dev_kfree_skb(skb);
@@ -388,6 +395,12 @@ static int submit_tx_urb(struct hfa384x *hw, struct urb
*tx_urb, gfp_t memflags)
struct net_device *netdev = hw->wlandev->netdev;
int result;
+ result = usb_urb_ep_type_check(&hw->tx_urb);
+ if (result) {
+ netdev_warn(hw->wlandev->netdev, "invalid tx endpoint");
+ goto done;
+ }
+
result = -ENOLINK;
if (netif_running(netdev)) {
if (!hw->wlandev->hwremoved &&
@@ -407,6 +420,7 @@ static int submit_tx_urb(struct hfa384x *hw, struct urb
*tx_urb, gfp_t memflags)
}
}
+done:
return result;
}
--
2.27.0
Re: [PATCH] staging: wlan-ng: properly check endpoint types
On Sun, Jul 19, 2020 at 11:23:38AM +0200, Greg KH wrote:
> On Sat, Jul 18, 2020 at 08:58:36AM -0700, Rustam Kovhaev wrote:
> > As syzkaller detected, wlan-ng driver submits bulk urb without checking
> > that the endpoint type is actually bulk, add usb_urb_ep_type_check()
> >
> > Reported-and-tested-by:
> > syzbot+c2a1fa67c02faa0de...@syzkaller.appspotmail.com
> > Link: https://syzkaller.appspot.com/bug?extid=c2a1fa67c02faa0de723
> > Signed-off-by: Rustam Kovhaev
> > ---
> > drivers/staging/wlan-ng/hfa384x_usb.c | 14 ++
> > 1 file changed, 14 insertions(+)
> >
> > diff --git a/drivers/staging/wlan-ng/hfa384x_usb.c
> > b/drivers/staging/wlan-ng/hfa384x_usb.c
> > index fa1bf8b069fd..7cde60ea68a2 100644
> > --- a/drivers/staging/wlan-ng/hfa384x_usb.c
> > +++ b/drivers/staging/wlan-ng/hfa384x_usb.c
> > @@ -339,6 +339,12 @@ static int submit_rx_urb(struct hfa384x *hw, gfp_t
> > memflags)
> >
> > hw->rx_urb_skb = skb;
> >
> > + result = usb_urb_ep_type_check(&hw->rx_urb);
> > + if (result) {
> > + netdev_warn(hw->wlandev->netdev, "invalid rx endpoint");
> > + goto cleanup;
> > + }
>
> In looking at this again, can you just make these checks in the probe
> function, and abort binding the driver to the device at that point in
> time? This feels really late in the init sequence.
>
> The real problem here is in the hfa384x_create() function, where it
> blindly takes the 1 and 2 endpoints and assumes that those are the
> "correct type". How about checking the types there, and if they are
> incorrect, returning an error from that function and have the caller
> return the error as well.
>
> That should keep anything else in the driver from being initialized and
> set up, and stop bad devices from being bound to the driver at a much
> earlier point in time.
>
> Note, just checking for the valid type/direction of those endpoints
> should be sufficient.
>
tyvm for the feedback! makes perfect sense, i'll send a new patch
[PATCH] usb: hso: check for return value in hso_serial_common_create()
in case of an error tty_register_device_attr() returns ERR_PTR(), add IS_ERR() check Reported-and-tested-by: syzbot+67b2bd0e34f952d03...@syzkaller.appspotmail.com Link: https://syzkaller.appspot.com/bug?extid=67b2bd0e34f952d0321e Signed-off-by: Rustam Kovhaev --- drivers/net/usb/hso.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c index 5f123a8cf68e..d2fdb5430d27 100644 --- a/drivers/net/usb/hso.c +++ b/drivers/net/usb/hso.c @@ -2261,12 +2261,14 @@ static int hso_serial_common_create(struct hso_serial *serial, int num_urbs, minor = get_free_serial_index(); if (minor < 0) - goto exit; + goto exit2; /* register our minor number */ serial->parent->dev = tty_port_register_device_attr(&serial->port, tty_drv, minor, &serial->parent->interface->dev, serial->parent, hso_serial_dev_groups); + if (IS_ERR(serial->parent->dev)) + goto exit2; /* fill in specific data for later use */ serial->minor = minor; @@ -2311,6 +2313,7 @@ static int hso_serial_common_create(struct hso_serial *serial, int num_urbs, return 0; exit: hso_serial_tty_unregister(serial); +exit2: hso_serial_common_free(serial); return -1; } -- 2.27.0
[PATCH] staging: wlan-ng: properly check endpoint types
As syzkaller detected, wlan-ng driver does not do sanity check of
endpoints in prism2sta_probe_usb(), add check for xfer direction and type
Reported-and-tested-by: syzbot+c2a1fa67c02faa0de...@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?extid=c2a1fa67c02faa0de723
Signed-off-by: Rustam Kovhaev
---
drivers/staging/wlan-ng/prism2usb.c | 16 +++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/wlan-ng/prism2usb.c
b/drivers/staging/wlan-ng/prism2usb.c
index 4689b2170e4f..456603fd26c0 100644
--- a/drivers/staging/wlan-ng/prism2usb.c
+++ b/drivers/staging/wlan-ng/prism2usb.c
@@ -61,11 +61,25 @@ static int prism2sta_probe_usb(struct usb_interface
*interface,
const struct usb_device_id *id)
{
struct usb_device *dev;
-
+ const struct usb_endpoint_descriptor *epd;
+ const struct usb_host_interface *iface_desc = interface->cur_altsetting;
struct wlandevice *wlandev = NULL;
struct hfa384x *hw = NULL;
int result = 0;
+ if (iface_desc->desc.bNumEndpoints != 2) {
+ result = -ENODEV;
+ goto failed;
+ }
+
+ result = -EINVAL;
+ epd = &iface_desc->endpoint[1].desc;
+ if (!usb_endpoint_is_bulk_in(epd))
+ goto failed;
+ epd = &iface_desc->endpoint[2].desc;
+ if (!usb_endpoint_is_bulk_out(epd))
+ goto failed;
+
dev = interface_to_usbdev(interface);
wlandev = create_wlan();
if (!wlandev) {
--
2.27.0
[PATCH] staging: rtl8712: handle firmware load failure
when firmware fails to load we should not call unregister_netdev()
this patch fixes a race condition between rtl871x_load_fw_cb() and
r871xu_dev_remove() and fixes the bug reported by syzbot
Reported-by: syzbot+80899a8a8efe8968c...@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?extid=80899a8a8efe8968cde7
Signed-off-by: Rustam Kovhaev
---
drivers/staging/rtl8712/hal_init.c | 3 ++-
drivers/staging/rtl8712/usb_intf.c | 11 ---
2 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/rtl8712/hal_init.c
b/drivers/staging/rtl8712/hal_init.c
index 40145c0338e4..42c0a3c947f1 100644
--- a/drivers/staging/rtl8712/hal_init.c
+++ b/drivers/staging/rtl8712/hal_init.c
@@ -33,7 +33,6 @@ static void rtl871x_load_fw_cb(const struct firmware
*firmware, void *context)
{
struct _adapter *adapter = context;
- complete(&adapter->rtl8712_fw_ready);
if (!firmware) {
struct usb_device *udev = adapter->dvobjpriv.pusbdev;
struct usb_interface *usb_intf = adapter->pusb_intf;
@@ -41,11 +40,13 @@ static void rtl871x_load_fw_cb(const struct firmware
*firmware, void *context)
dev_err(&udev->dev, "r8712u: Firmware request failed\n");
usb_put_dev(udev);
usb_set_intfdata(usb_intf, NULL);
+ complete(&adapter->rtl8712_fw_ready);
return;
}
adapter->fw = firmware;
/* firmware available - start netdev */
register_netdev(adapter->pnetdev);
+ complete(&adapter->rtl8712_fw_ready);
}
static const char firmware_file[] = "rtlwifi/rtl8712u.bin";
diff --git a/drivers/staging/rtl8712/usb_intf.c
b/drivers/staging/rtl8712/usb_intf.c
index a87562f632a7..2fcd65260f4c 100644
--- a/drivers/staging/rtl8712/usb_intf.c
+++ b/drivers/staging/rtl8712/usb_intf.c
@@ -595,13 +595,17 @@ static void r871xu_dev_remove(struct usb_interface
*pusb_intf)
if (pnetdev) {
struct _adapter *padapter = netdev_priv(pnetdev);
- usb_set_intfdata(pusb_intf, NULL);
- release_firmware(padapter->fw);
/* never exit with a firmware callback pending */
wait_for_completion(&padapter->rtl8712_fw_ready);
+ pnetdev = usb_get_intfdata(pusb_intf);
+ usb_set_intfdata(pusb_intf, NULL);
+ if (!pnetdev)
+ goto firmware_load_fail;
+ release_firmware(padapter->fw);
if (drvpriv.drv_registered)
padapter->surprise_removed = true;
- unregister_netdev(pnetdev); /* will call netdev_close() */
+ if (pnetdev->reg_state != NETREG_UNINITIALIZED)
+ unregister_netdev(pnetdev); /* will call netdev_close()
*/
flush_scheduled_work();
udelay(1);
/* Stop driver mlme relation timer */
@@ -614,6 +618,7 @@ static void r871xu_dev_remove(struct usb_interface
*pusb_intf)
*/
usb_put_dev(udev);
}
+firmware_load_fail:
/* If we didn't unplug usb dongle and remove/insert module, driver
* fails on sitesurvey for the first time when device is up.
* Reset usb port for sitesurvey fail issue.
--
2.27.0
[PATCH] KVM: use struct_size() and flex_array_size() helpers in kvm_io_bus_unregister_dev()
Make use of the struct_size() helper to avoid any potential type
mistakes and protect against potential integer overflows
Make use of the flex_array_size() helper to calculate the size of a
flexible array member within an enclosing structure
Cc: sta...@vger.kernel.org
Suggested-by: Gustavo A. R. Silva
Signed-off-by: Rustam Kovhaev
---
virt/kvm/kvm_main.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index cf88233b819a..68edd25dcb11 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -4350,10 +4350,10 @@ void kvm_io_bus_unregister_dev(struct kvm *kvm, enum
kvm_bus bus_idx,
new_bus = kmalloc(struct_size(bus, range, bus->dev_count - 1),
GFP_KERNEL_ACCOUNT);
if (new_bus) {
- memcpy(new_bus, bus, sizeof(*bus) + i * sizeof(struct
kvm_io_range));
+ memcpy(new_bus, bus, struct_size(bus, range, i));
new_bus->dev_count--;
memcpy(new_bus->range + i, bus->range + i + 1,
- (new_bus->dev_count - i) * sizeof(struct kvm_io_range));
+ flex_array_size(new_bus, range,
new_bus->dev_count - i));
} else {
pr_err("kvm: failed to shrink bus, removing it completely\n");
for (j = 0; j < bus->dev_count; j++) {
--
2.28.0
[RESEND PATCH] KVM: use struct_size() and flex_array_size() helpers in kvm_io_bus_unregister_dev()
Make use of the struct_size() helper to avoid any potential type
mistakes and protect against potential integer overflows
Make use of the flex_array_size() helper to calculate the size of a
flexible array member within an enclosing structure
Suggested-by: Gustavo A. R. Silva
Signed-off-by: Rustam Kovhaev
Reviewed-by: Gustavo A. R. Silva
---
virt/kvm/kvm_main.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index cf88233b819a..68edd25dcb11 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -4350,10 +4350,10 @@ void kvm_io_bus_unregister_dev(struct kvm *kvm, enum
kvm_bus bus_idx,
new_bus = kmalloc(struct_size(bus, range, bus->dev_count - 1),
GFP_KERNEL_ACCOUNT);
if (new_bus) {
- memcpy(new_bus, bus, sizeof(*bus) + i * sizeof(struct
kvm_io_range));
+ memcpy(new_bus, bus, struct_size(bus, range, i));
new_bus->dev_count--;
memcpy(new_bus->range + i, bus->range + i + 1,
- (new_bus->dev_count - i) * sizeof(struct kvm_io_range));
+ flex_array_size(new_bus, range,
new_bus->dev_count - i));
} else {
pr_err("kvm: failed to shrink bus, removing it completely\n");
for (j = 0; j < bus->dev_count; j++) {
--
2.28.0
