On Fri, Nov 18, 2022 at 04:07:24PM -0500, Alan Stern wrote: > On Fri, Nov 18, 2022 at 04:37:32PM +0000, John Keeping wrote: > > I don't think it's at all simple to fix this - I posted a series > > addressing the lifetime issues here a few years ago but didn't chase it > > up and there was no feedback: > > > > https://lore.kernel.org/linux-usb/20191028114228.3679219-1-john@xxxxxxxxxxxx/ > > > > That includes a patch to remove the embedded struct cdev and manage its > > lifetime separately, which I think is needed as there are two different > > struct device objects here and we cannot tie their lifetimes together. > > I still don't have a clear picture of what the real problem is. Lee's > original patch description just said "external references are presently > not tracked", with no details about what those external references are. > Why not add just proper cdev_get() and cdev_put() calls to whatever code > handles those external references, so that they _are_ tracked? > > What are the two different struct device objects? Why do their > lifetimes need to be tied together? If you do need to tie their > lifetimes somehow, why not simply make one of them (the one which is > logically allowed to be shorter-lived) hold a reference to the other? The problem is that we have a struct cdev embedded in f_hidg but the lifetime of f_hidg is not tied to any kobject so we can't solve this in the right way by setting the parent kobject of the cdev. While refcounting struct f_hidg is necessary, it's not sufficient because the only way to keep it alive long enough for the final kobject_put() on the embedded cdev is to tie the lifetime to a kobject of its own and there is no suitable object as this is not the model followed by gadget function instances. To show the problem (using libusbgx's example commands for conciseness, but obviously this can be done via configfs directly): $ gadget-hid $ exec 3<> /dev/hidg0 $ gadget-vid-pid-remove $ exec 3<&- ================================================================== BUG: KASAN: use-after-free in kobject_put+0x24/0x250 Read of size 1 at addr c61784a0 by task sh/264 CPU: 1 PID: 264 Comm: sh Tainted: G W 6.0.5 #1 Hardware name: Rockchip (Device Tree) unwind_backtrace from show_stack+0x10/0x14 show_stack from dump_stack_lvl+0x58/0x70 dump_stack_lvl from print_report+0x58/0x4dc print_report from kasan_report+0x88/0xa8 kasan_report from kobject_put+0x24/0x250 kobject_put from __fput+0x1e4/0x358 __fput from task_work_run+0xb4/0xc4 task_work_run from do_work_pending+0x4d4/0x524 do_work_pending from slow_work_pending+0xc/0x20 Exception stack(0xf284bfb0 to 0xf284bff8) bfa0: 00000000 00000003 02292190 00000000 bfc0: 02293fc8 aed4e1d0 00000001 00000006 022925a0 00000000 022925a0 022923f4 bfe0: 00532f38 b6864840 0049c218 aec57e38 60000010 0000000b Allocated by task 341: kasan_set_track+0x20/0x28 ____kasan_kmalloc+0x80/0x88 hidg_alloc+0x24/0x1f0 usb_get_function+0x28/0x48 config_usb_cfg_link+0x90/0x114 configfs_symlink+0x24c/0x5d8 vfs_symlink+0x58/0x74 do_symlinkat+0xdc/0x16c ret_fast_syscall+0x0/0x1c Freed by task 352: kasan_set_track+0x20/0x28 kasan_set_free_info+0x28/0x34 ____kasan_slab_free+0xf8/0x108 __kasan_slab_free+0x10/0x18 slab_free_freelist_hook+0x9c/0xfc kfree+0x118/0x258 hidg_free+0x44/0x6c config_usb_cfg_unlink+0xd4/0xf4 configfs_unlink+0x118/0x15c vfs_unlink+0xd8/0x1c4 do_unlinkat+0x180/0x28c ret_fast_syscall+0x0/0x1c The buggy address belongs to the object at c6178400 which belongs to the cache kmalloc-512 of size 512 The buggy address is located 160 bytes inside of 512-byte region [c6178400, c6178600)
