On Wed, Oct 28, 2020 at 4:13 PM Kyungtae Kim <kt0755@xxxxxxxxx> wrote: > > FuzzUSB (a variant of syzkaller) found the bug > when accessing a freed instance of struct f_hidg. > > Reference: https://www.spinics.net/lists/linux-usb/msg195103.html > > The fix uses reference count to ensure the right access to instance of f_hidg. > > > BUG: KASAN: use-after-free in f_hidg_poll+0x190/0x1e0 drivers/usb/gadget/function/f_hid.c:424 > Read of size 1 at addr ffff8880579260e8 by task syz-executor.5/2849 > > CPU: 3 PID: 2849 Comm: syz-executor.5 Not tainted 5.6.11 #1 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 > Call Trace: > __dump_stack lib/dump_stack.c:77 [inline] > dump_stack+0xce/0x128 lib/dump_stack.c:118 > print_address_description.constprop.4+0x21/0x3c0 mm/kasan/report.c:374 > __kasan_report+0x131/0x1b0 mm/kasan/report.c:506 > kasan_report+0x12/0x20 mm/kasan/common.c:641 > __asan_report_load1_noabort+0x14/0x20 mm/kasan/generic_report.c:132 > f_hidg_poll+0x190/0x1e0 drivers/usb/gadget/function/f_hid.c:424 > vfs_poll include/linux/poll.h:90 [inline] > do_pollfd fs/select.c:859 [inline] > do_poll fs/select.c:907 [inline] > do_sys_poll+0x548/0xe20 fs/select.c:1001 > __do_sys_poll fs/select.c:1059 [inline] > __se_sys_poll fs/select.c:1047 [inline] > __x64_sys_poll+0x171/0x420 fs/select.c:1047 > do_syscall_64+0x9e/0x510 arch/x86/entry/common.c:294 > entry_SYSCALL_64_after_hwframe+0x49/0xbe > RIP: 0033:0x4531a9 > Code: ed 60 fc ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 bb 60 fc ff c3 66 2e 0f 1f 84 00 00 00 00 > RSP: 002b:00007f07bfcd1c78 EFLAGS: 00000246 ORIG_RAX: 0000000000000007 > RAX: ffffffffffffffda RBX: 000000000073bfa8 RCX: 00000000004531a9 > RDX: 0000000000000080 RSI: 0000000000000001 RDI: 0000000020001980 > RBP: 0000000000000003 R08: 0000000000000000 R09: 0000000000000000 > R10: 0000000000000000 R11: 0000000000000246 R12: 00000000004bd290 > R13: 00000000004d2c28 R14: 00007f07bfcd26d4 R15: 00000000ffffffff > > Allocated by task 2418: > save_stack+0x21/0x90 mm/kasan/common.c:72 > set_track mm/kasan/common.c:80 [inline] > __kasan_kmalloc.constprop.3+0xa7/0xd0 mm/kasan/common.c:515 > kasan_kmalloc+0x9/0x10 mm/kasan/common.c:529 > kmem_cache_alloc_trace+0xfa/0x2d0 mm/slub.c:2813 > kzalloc include/linux/slab.h:555 [inline] > hidg_alloc+0x56/0x5e0 drivers/usb/gadget/function/f_hid.c:1091 > usb_get_function+0x58/0xc0 drivers/usb/gadget/functions.c:61 > config_usb_cfg_link+0x1ed/0x3e0 drivers/usb/gadget/configfs.c:444 > configfs_symlink+0x527/0x11d0 fs/configfs/symlink.c:202 > vfs_symlink+0x33d/0x5b0 fs/namei.c:4201 > do_symlinkat+0x11b/0x1d0 fs/namei.c:4228 > __do_sys_symlinkat fs/namei.c:4242 [inline] > __se_sys_symlinkat fs/namei.c:4239 [inline] > __x64_sys_symlinkat+0x73/0xb0 fs/namei.c:4239 > do_syscall_64+0x9e/0x510 arch/x86/entry/common.c:294 > entry_SYSCALL_64_after_hwframe+0x49/0xbe > > Freed by task 2868: > save_stack+0x21/0x90 mm/kasan/common.c:72 > set_track mm/kasan/common.c:80 [inline] > kasan_set_free_info mm/kasan/common.c:337 [inline] > __kasan_slab_free+0x135/0x190 mm/kasan/common.c:476 > kasan_slab_free+0xe/0x10 mm/kasan/common.c:485 > slab_free_hook mm/slub.c:1444 [inline] > slab_free_freelist_hook mm/slub.c:1477 [inline] > slab_free mm/slub.c:3034 [inline] > kfree+0xf7/0x410 mm/slub.c:3995 > hidg_free+0x7f/0x110 drivers/usb/gadget/function/f_hid.c:1069 > usb_put_function+0x38/0x50 drivers/usb/gadget/functions.c:87 > config_usb_cfg_unlink+0x2db/0x3b0 drivers/usb/gadget/configfs.c:485 > configfs_unlink+0x3b9/0x7f0 fs/configfs/symlink.c:250 > vfs_unlink+0x287/0x570 fs/namei.c:4073 > do_unlinkat+0x4f9/0x620 fs/namei.c:4137 > __do_sys_unlink fs/namei.c:4184 [inline] > __se_sys_unlink fs/namei.c:4182 [inline] > __x64_sys_unlink+0x42/0x50 fs/namei.c:4182 > do_syscall_64+0x9e/0x510 arch/x86/entry/common.c:294 > entry_SYSCALL_64_after_hwframe+0x49/0xbe > > > Signed-off-by: Kyungtae Kim <kt0755@xxxxxxxxx> > Reported-and-tested-by: Kyungtae Kim <kt0755@xxxxxxxxx> > > --- > drivers/usb/gadget/function/f_hid.c | 21 ++++++++++++++++++++- > 1 file changed, 20 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/gadget/function/f_hid.c b/drivers/usb/gadget/function/f_hid.c > index 1125f4715830..e900b51c075a 100644 > --- a/drivers/usb/gadget/function/f_hid.c > +++ b/drivers/usb/gadget/function/f_hid.c > @@ -16,6 +16,7 @@ > #include <linux/wait.h> > #include <linux/sched.h> > #include <linux/usb/g_hid.h> > +#include <linux/kref.h> > > #include "u_f.h" > #include "u_hid.h" > @@ -44,6 +45,7 @@ struct f_hidg { > unsigned short report_desc_length; > char *report_desc; > unsigned short report_length; > + struct kref kref; > > /* recv report */ > struct list_head completed_out_req; > @@ -427,9 +429,21 @@ static __poll_t f_hidg_poll(struct file *file, poll_table *wait) > #undef WRITE_COND > #undef READ_COND > > +static void f_hidg_free(struct kref *kref) > +{ > + struct f_hidg *hidg = container_of(kref, struct f_hidg, kref); > + > + kfree(hidg); > +} > + > static int f_hidg_release(struct inode *inode, struct file *fd) > { > + struct f_hidg *hidg = fd->private_data; > + > fd->private_data = NULL; > + > + kref_put(&hidg->kref, f_hidg_free); > + > return 0; > } > > @@ -440,6 +454,8 @@ static int f_hidg_open(struct inode *inode, struct file *fd) > > fd->private_data = hidg; > > + kref_get(&hidg->kref); > + > return 0; > } > > @@ -1060,7 +1076,9 @@ static void hidg_free(struct usb_function *f) > hidg = func_to_hidg(f); > opts = container_of(f->fi, struct f_hid_opts, func_inst); > kfree(hidg->report_desc); > - kfree(hidg); > + > + kref_put(&hidg->kref, f_hidg_free); > + > mutex_lock(&opts->lock); > --opts->refcnt; > mutex_unlock(&opts->lock); > @@ -1089,6 +1107,7 @@ static struct usb_function *hidg_alloc(struct usb_function_instance *fi) > opts = container_of(fi, struct f_hid_opts, func_inst); > > mutex_lock(&opts->lock); > + kref_init(&hidg->kref); > ++opts->refcnt; > > hidg->minor = opts->minor; > -- > 2.17.1 >
