On Mon, Oct 07, 2024 at 02:07:43PM +0300, Heikki Krogerus wrote:
> On Fri, Oct 04, 2024 at 09:37:38AM -0300, Thadeu Lima de Souza Cascardo wrote:
> > The altmode device release refers to its parent device, but without keeping
> > a reference to it.
> >
> > When registering the altmode, get a reference to the parent and put it in
> > the release function.
> >
> > Before this fix, when using CONFIG_DEBUG_KOBJECT_RELEASE, we see issues
> > like this:
> >
> > [ 43.572860] kobject: 'port0.0' (ffff8880057ba008): kobject_release, parent 0000000000000000 (delayed 3000)
> > [ 43.573532] kobject: 'port0.1' (ffff8880057bd008): kobject_release, parent 0000000000000000 (delayed 1000)
> > [ 43.574407] kobject: 'port0' (ffff8880057b9008): kobject_release, parent 0000000000000000 (delayed 3000)
> > [ 43.575059] kobject: 'port1.0' (ffff8880057ca008): kobject_release, parent 0000000000000000 (delayed 4000)
> > [ 43.575908] kobject: 'port1.1' (ffff8880057c9008): kobject_release, parent 0000000000000000 (delayed 4000)
> > [ 43.576908] kobject: 'typec' (ffff8880062dbc00): kobject_release, parent 0000000000000000 (delayed 4000)
> > [ 43.577769] kobject: 'port1' (ffff8880057bf008): kobject_release, parent 0000000000000000 (delayed 3000)
> > [ 46.612867] ==================================================================
> > [ 46.613402] BUG: KASAN: slab-use-after-free in typec_altmode_release+0x38/0x129
> > [ 46.614003] Read of size 8 at addr ffff8880057b9118 by task kworker/2:1/48
> > [ 46.614538]
> > [ 46.614668] CPU: 2 UID: 0 PID: 48 Comm: kworker/2:1 Not tainted 6.12.0-rc1-00138-gedbae730ad31 #535
> > [ 46.615391] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.15.0-1 04/01/2014
> > [ 46.616042] Workqueue: events kobject_delayed_cleanup
> > [ 46.616446] Call Trace:
> > [ 46.616648] <TASK>
> > [ 46.616820] dump_stack_lvl+0x5b/0x7c
> > [ 46.617112] ? typec_altmode_release+0x38/0x129
> > [ 46.617470] print_report+0x14c/0x49e
> > [ 46.617769] ? rcu_read_unlock_sched+0x56/0x69
> > [ 46.618117] ? __virt_addr_valid+0x19a/0x1ab
> > [ 46.618456] ? kmem_cache_debug_flags+0xc/0x1d
> > [ 46.618807] ? typec_altmode_release+0x38/0x129
> > [ 46.619161] kasan_report+0x8d/0xb4
> > [ 46.619447] ? typec_altmode_release+0x38/0x129
> > [ 46.619809] ? process_scheduled_works+0x3cb/0x85f
> > [ 46.620185] typec_altmode_release+0x38/0x129
> > [ 46.620537] ? process_scheduled_works+0x3cb/0x85f
> > [ 46.620907] device_release+0xaf/0xf2
> > [ 46.621206] kobject_delayed_cleanup+0x13b/0x17a
> > [ 46.621584] process_scheduled_works+0x4f6/0x85f
> > [ 46.621955] ? __pfx_process_scheduled_works+0x10/0x10
> > [ 46.622353] ? hlock_class+0x31/0x9a
> > [ 46.622647] ? lock_acquired+0x361/0x3c3
> > [ 46.622956] ? move_linked_works+0x46/0x7d
> > [ 46.623277] worker_thread+0x1ce/0x291
> > [ 46.623582] ? __kthread_parkme+0xc8/0xdf
> > [ 46.623900] ? __pfx_worker_thread+0x10/0x10
> > [ 46.624236] kthread+0x17e/0x190
> > [ 46.624501] ? kthread+0xfb/0x190
> > [ 46.624756] ? __pfx_kthread+0x10/0x10
> > [ 46.625015] ret_from_fork+0x20/0x40
> > [ 46.625268] ? __pfx_kthread+0x10/0x10
> > [ 46.625532] ret_from_fork_asm+0x1a/0x30
> > [ 46.625805] </TASK>
> > [ 46.625953]
> > [ 46.626056] Allocated by task 678:
> > [ 46.626287] kasan_save_stack+0x24/0x44
> > [ 46.626555] kasan_save_track+0x14/0x2d
> > [ 46.626811] __kasan_kmalloc+0x3f/0x4d
> > [ 46.627049] __kmalloc_noprof+0x1bf/0x1f0
> > [ 46.627362] typec_register_port+0x23/0x491
> > [ 46.627698] cros_typec_probe+0x634/0xbb6
> > [ 46.628026] platform_probe+0x47/0x8c
> > [ 46.628311] really_probe+0x20a/0x47d
> > [ 46.628605] device_driver_attach+0x39/0x72
> > [ 46.628940] bind_store+0x87/0xd7
> > [ 46.629213] kernfs_fop_write_iter+0x1aa/0x218
> > [ 46.629574] vfs_write+0x1d6/0x29b
> > [ 46.629856] ksys_write+0xcd/0x13b
> > [ 46.630128] do_syscall_64+0xd4/0x139
> > [ 46.630420] entry_SYSCALL_64_after_hwframe+0x76/0x7e
> > [ 46.630820]
> > [ 46.630946] Freed by task 48:
> > [ 46.631182] kasan_save_stack+0x24/0x44
> > [ 46.631493] kasan_save_track+0x14/0x2d
> > [ 46.631799] kasan_save_free_info+0x3f/0x4d
> > [ 46.632144] __kasan_slab_free+0x37/0x45
> > [ 46.632474] kfree+0x1d4/0x252
> > [ 46.632725] device_release+0xaf/0xf2
> > [ 46.633017] kobject_delayed_cleanup+0x13b/0x17a
> > [ 46.633388] process_scheduled_works+0x4f6/0x85f
> > [ 46.633764] worker_thread+0x1ce/0x291
> > [ 46.634065] kthread+0x17e/0x190
> > [ 46.634324] ret_from_fork+0x20/0x40
> > [ 46.634621] ret_from_fork_asm+0x1a/0x30
> >
> > Fixes: 8a37d87d72f0 ("usb: typec: Bus type for alternate modes")
> > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@xxxxxxxxxx>
>
> I tried to go through the code and the logs, but I did not really find
> any explanation why we couldn't keep the device release order in sync
> for all devices that need it. I'll see if we can improve that somehow
> in the core, but we can fix this one in the meantime.
>
> But this is a case of use-after-free, so shouldn't this go to the
> stable trees?
Agreed. I am fine if Cc: stable@xxxxxxxxxxxxxxx get added when this is
applied. But otherwise, I can keep track in case the Fixes line does not
trigger its inclusion.
Thanks.
Cascardo.
>
> Reviewed-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
>
> thanks,
>
> --
> heikki
>
