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:
Let me study what's going on in the drivers code. The children should
_not_ be cleaned first before the parent. I'll have to come back to
this on Monday.
This really should not be necessary.
> [ 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>
> ---
> drivers/usb/typec/class.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index 9262fcd4144f..d61b4c74648d 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -519,6 +519,7 @@ static void typec_altmode_release(struct device *dev)
> typec_altmode_put_partner(alt);
>
> altmode_id_remove(alt->adev.dev.parent, alt->id);
> + put_device(alt->adev.dev.parent);
> kfree(alt);
> }
>
> @@ -568,6 +569,8 @@ typec_register_altmode(struct device *parent,
> alt->adev.dev.type = &typec_altmode_dev_type;
> dev_set_name(&alt->adev.dev, "%s.%u", dev_name(parent), id);
>
> + get_device(alt->adev.dev.parent);
> +
> /* Link partners and plugs with the ports */
> if (!is_port)
> typec_altmode_set_partner(alt);
> --
> 2.34.1
--
heikki
