Re: [PATCH] USB: gadgetfs, dummy-hcd, net2280: fix locking for callbacks

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> writes:
> On Tue, Jun 13, 2017 at 03:23:42PM -0400, Alan Stern wrote:
>> Using the syzkaller kernel fuzzer, Andrey Konovalov generated the
>> following error in gadgetfs:
>> 
>> > BUG: KASAN: use-after-free in __lock_acquire+0x3069/0x3690
>> > kernel/locking/lockdep.c:3246
>> > Read of size 8 at addr ffff88003a2bdaf8 by task kworker/3:1/903
>> > 
>> > CPU: 3 PID: 903 Comm: kworker/3:1 Not tainted 4.12.0-rc4+ #35
>> > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>> > Workqueue: usb_hub_wq hub_event
>> > Call Trace:
>> >  __dump_stack lib/dump_stack.c:16 [inline]
>> >  dump_stack+0x292/0x395 lib/dump_stack.c:52
>> >  print_address_description+0x78/0x280 mm/kasan/report.c:252
>> >  kasan_report_error mm/kasan/report.c:351 [inline]
>> >  kasan_report+0x230/0x340 mm/kasan/report.c:408
>> >  __asan_report_load8_noabort+0x19/0x20 mm/kasan/report.c:429
>> >  __lock_acquire+0x3069/0x3690 kernel/locking/lockdep.c:3246
>> >  lock_acquire+0x22d/0x560 kernel/locking/lockdep.c:3855
>> >  __raw_spin_lock include/linux/spinlock_api_smp.h:142 [inline]
>> >  _raw_spin_lock+0x2f/0x40 kernel/locking/spinlock.c:151
>> >  spin_lock include/linux/spinlock.h:299 [inline]
>> >  gadgetfs_suspend+0x89/0x130 drivers/usb/gadget/legacy/inode.c:1682
>> >  set_link_state+0x88e/0xae0 drivers/usb/gadget/udc/dummy_hcd.c:455
>> >  dummy_hub_control+0xd7e/0x1fb0 drivers/usb/gadget/udc/dummy_hcd.c:2074
>> >  rh_call_control drivers/usb/core/hcd.c:689 [inline]
>> >  rh_urb_enqueue drivers/usb/core/hcd.c:846 [inline]
>> >  usb_hcd_submit_urb+0x92f/0x20b0 drivers/usb/core/hcd.c:1650
>> >  usb_submit_urb+0x8b2/0x12c0 drivers/usb/core/urb.c:542
>> >  usb_start_wait_urb+0x148/0x5b0 drivers/usb/core/message.c:56
>> >  usb_internal_control_msg drivers/usb/core/message.c:100 [inline]
>> >  usb_control_msg+0x341/0x4d0 drivers/usb/core/message.c:151
>> >  usb_clear_port_feature+0x74/0xa0 drivers/usb/core/hub.c:412
>> >  hub_port_disable+0x123/0x510 drivers/usb/core/hub.c:4177
>> >  hub_port_init+0x1ed/0x2940 drivers/usb/core/hub.c:4648
>> >  hub_port_connect drivers/usb/core/hub.c:4826 [inline]
>> >  hub_port_connect_change drivers/usb/core/hub.c:4999 [inline]
>> >  port_event drivers/usb/core/hub.c:5105 [inline]
>> >  hub_event+0x1ae1/0x3d40 drivers/usb/core/hub.c:5185
>> >  process_one_work+0xc08/0x1bd0 kernel/workqueue.c:2097
>> >  process_scheduled_works kernel/workqueue.c:2157 [inline]
>> >  worker_thread+0xb2b/0x1860 kernel/workqueue.c:2233
>> >  kthread+0x363/0x440 kernel/kthread.c:231
>> >  ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:424
>> > 
>> > Allocated by task 9958:
>> >  save_stack_trace+0x1b/0x20 arch/x86/kernel/stacktrace.c:59
>> >  save_stack+0x43/0xd0 mm/kasan/kasan.c:513
>> >  set_track mm/kasan/kasan.c:525 [inline]
>> >  kasan_kmalloc+0xad/0xe0 mm/kasan/kasan.c:617
>> >  kmem_cache_alloc_trace+0x87/0x280 mm/slub.c:2745
>> >  kmalloc include/linux/slab.h:492 [inline]
>> >  kzalloc include/linux/slab.h:665 [inline]
>> >  dev_new drivers/usb/gadget/legacy/inode.c:170 [inline]
>> >  gadgetfs_fill_super+0x24f/0x540 drivers/usb/gadget/legacy/inode.c:1993
>> >  mount_single+0xf6/0x160 fs/super.c:1192
>> >  gadgetfs_mount+0x31/0x40 drivers/usb/gadget/legacy/inode.c:2019
>> >  mount_fs+0x9c/0x2d0 fs/super.c:1223
>> >  vfs_kern_mount.part.25+0xcb/0x490 fs/namespace.c:976
>> >  vfs_kern_mount fs/namespace.c:2509 [inline]
>> >  do_new_mount fs/namespace.c:2512 [inline]
>> >  do_mount+0x41b/0x2d90 fs/namespace.c:2834
>> >  SYSC_mount fs/namespace.c:3050 [inline]
>> >  SyS_mount+0xb0/0x120 fs/namespace.c:3027
>> >  entry_SYSCALL_64_fastpath+0x1f/0xbe
>> > 
>> > Freed by task 9960:
>> >  save_stack_trace+0x1b/0x20 arch/x86/kernel/stacktrace.c:59
>> >  save_stack+0x43/0xd0 mm/kasan/kasan.c:513
>> >  set_track mm/kasan/kasan.c:525 [inline]
>> >  kasan_slab_free+0x72/0xc0 mm/kasan/kasan.c:590
>> >  slab_free_hook mm/slub.c:1357 [inline]
>> >  slab_free_freelist_hook mm/slub.c:1379 [inline]
>> >  slab_free mm/slub.c:2961 [inline]
>> >  kfree+0xed/0x2b0 mm/slub.c:3882
>> >  put_dev+0x124/0x160 drivers/usb/gadget/legacy/inode.c:163
>> >  gadgetfs_kill_sb+0x33/0x60 drivers/usb/gadget/legacy/inode.c:2027
>> >  deactivate_locked_super+0x8d/0xd0 fs/super.c:309
>> >  deactivate_super+0x21e/0x310 fs/super.c:340
>> >  cleanup_mnt+0xb7/0x150 fs/namespace.c:1112
>> >  __cleanup_mnt+0x1b/0x20 fs/namespace.c:1119
>> >  task_work_run+0x1a0/0x280 kernel/task_work.c:116
>> >  exit_task_work include/linux/task_work.h:21 [inline]
>> >  do_exit+0x18a8/0x2820 kernel/exit.c:878
>> >  do_group_exit+0x14e/0x420 kernel/exit.c:982
>> >  get_signal+0x784/0x1780 kernel/signal.c:2318
>> >  do_signal+0xd7/0x2130 arch/x86/kernel/signal.c:808
>> >  exit_to_usermode_loop+0x1ac/0x240 arch/x86/entry/common.c:157
>> >  prepare_exit_to_usermode arch/x86/entry/common.c:194 [inline]
>> >  syscall_return_slowpath+0x3ba/0x410 arch/x86/entry/common.c:263
>> >  entry_SYSCALL_64_fastpath+0xbc/0xbe
>> > 
>> > The buggy address belongs to the object at ffff88003a2bdae0
>> >  which belongs to the cache kmalloc-1024 of size 1024
>> > The buggy address is located 24 bytes inside of
>> >  1024-byte region [ffff88003a2bdae0, ffff88003a2bdee0)
>> > The buggy address belongs to the page:
>> > page:ffffea0000e8ae00 count:1 mapcount:0 mapping:          (null)
>> > index:0x0 compound_mapcount: 0
>> > flags: 0x100000000008100(slab|head)
>> > raw: 0100000000008100 0000000000000000 0000000000000000 0000000100170017
>> > raw: ffffea0000ed3020 ffffea0000f5f820 ffff88003e80efc0 0000000000000000
>> > page dumped because: kasan: bad access detected
>> > 
>> > Memory state around the buggy address:
>> >  ffff88003a2bd980: fb fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>> >  ffff88003a2bda00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>> > >ffff88003a2bda80: fc fc fc fc fc fc fc fc fc fc fc fc fb fb fb fb
>> >                                                                 ^
>> >  ffff88003a2bdb00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>> >  ffff88003a2bdb80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>> > ==================================================================
>> 
>> What this means is that the gadgetfs_suspend() routine was trying to
>> access dev->lock after it had been deallocated.  The root cause is a
>> race in the dummy_hcd driver; the dummy_udc_stop() routine can race
>> with the rest of the driver because it contains no locking.  And even
>> when proper locking is added, it can still race with the
>> set_link_state() function because that function incorrectly drops the
>> private spinlock before invoking any gadget driver callbacks.
>> 
>> The result of this race, as seen above, is that set_link_state() can
>> invoke a callback in gadgetfs even after gadgetfs has been unbound
>> from dummy_hcd's UDC and its private data structures have been
>> deallocated.
>> 
>> include/linux/usb/gadget.h documents that the ->reset, ->disconnect,
>> ->suspend, and ->resume callbacks may be invoked in interrupt context.
>> In general this is necessary, to prevent races with gadget driver
>> removal.  This patch fixes dummy_hcd to retain the spinlock across
>> these calls, and it adds a spinlock acquisition to dummy_udc_stop() to
>> prevent the race.
>> 
>> The net2280 driver makes the same mistake of dropping the private
>> spinlock for its ->disconnect and ->reset callback invocations.  The
>> patch fixes it too.
>> 
>> Lastly, since gadgetfs_suspend() may be invoked in interrupt context,
>> it cannot assume that interrupts are enabled when it runs.  It must
>> use spin_lock_irqsave() instead of spin_lock_irq().  The patch fixes
>> that bug as well.
>> 
>> Signed-off-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
>> Reported-and-tested-by: Andrey Konovalov <andreyknvl@xxxxxxxxxx>
>> CC: <stable@xxxxxxxxxxxxxxx>
>> 
>> ---
>
> Felipe, want me to take this directly in my tree now?

please do. I don't have that HW for testing anyway :-(

Acked-by: Felipe Balbi <felipe.balbi@xxxxxxxxxxxxxxx>

Thanks a lot :-)

-- 
balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux