Re: [PATCH] clk: fix slab-use-after-free when clk_core_populate_parent_map failed

[Zhou, Yun]

Hi Brian,

Thanks for your review.

This defect is caused by multiple reasons, but as you said, the fixes 
tag is not accurate enough.

I think it should be:

Fixes: fc0c209c1 ("clk: Allow parents to be specified without string names")

Do you think so?

Regards,

Yun


On 9/30/25 03:54, Brian Masney wrote:
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
> On Mon, Sep 29, 2025 at 04:31:19PM +0800, Yun Zhou wrote:
> > If clk_core_populate_parent_map() fails, core->parents will be immediately
> > released within clk_core_populate_parent_map(). Therefore it is can't be
> > released in __clk_release() again.
> >
> > This fixes the following KASAN reported issue:
> >
> > ==================================================================
> > BUG: KASAN: slab-use-after-free in __clk_release+0x80/0x160
> > Read of size 8 at addr ffffff8043fd0980 by task kworker/u6:0/27
> >
> > CPU: 1 PID: 27 Comm: kworker/u6:0 Tainted: G        W          6.6.69-yocto-standard+ #7
> > Hardware name: Raspberry Pi 4 Model B (DT)
> > Workqueue: events_unbound deferred_probe_work_func
> > Call trace:
> >   dump_backtrace+0x98/0xf8
> >   show_stack+0x20/0x38
> >   dump_stack_lvl+0x48/0x60
> >   print_report+0xf8/0x5d8
> >   kasan_report+0xb4/0x100
> >   __asan_load8+0x9c/0xc0
> >   __clk_release+0x80/0x160
> >   __clk_register+0x6dc/0xfb8
> >   devm_clk_hw_register+0x70/0x108
> >   bcm2835_register_clock+0x284/0x358
> >   bcm2835_clk_probe+0x2c4/0x438
> >   platform_probe+0x98/0x110
> >   really_probe+0x1e4/0x3e8
> >   __driver_probe_device+0xc0/0x1a0
> >   driver_probe_device+0x110/0x1e8
> >   __device_attach_driver+0xf0/0x1a8
> >   bus_for_each_drv+0xf8/0x178
> >   __device_attach+0x120/0x240
> >   device_initial_probe+0x1c/0x30
> >   bus_probe_device+0xdc/0xe8
> >   deferred_probe_work_func+0xe8/0x130
> >   process_one_work+0x2a4/0x698
> >   worker_thread+0x53c/0x708
> >   kthread+0x1b4/0x1c8
> >   ret_from_fork+0x10/0x20
> >
> > Allocated by task 27:
> >   kasan_save_stack+0x3c/0x68
> >   kasan_set_track+0x2c/0x40
> >   kasan_save_alloc_info+0x24/0x38
> >   __kasan_kmalloc+0xd4/0xd8
> >   __kmalloc+0x74/0x238
> >   __clk_register+0x718/0xfb8
> >   devm_clk_hw_register+0x70/0x108
> >   bcm2835_register_clock+0x284/0x358
> >   bcm2835_clk_probe+0x2c4/0x438
> >   platform_probe+0x98/0x110
> >   really_probe+0x1e4/0x3e8
> >   __driver_probe_device+0xc0/0x1a0
> >   driver_probe_device+0x110/0x1e8
> >   __device_attach_driver+0xf0/0x1a8
> >   bus_for_each_drv+0xf8/0x178
> >   __device_attach+0x120/0x240
> >   device_initial_probe+0x1c/0x30
> >   bus_probe_device+0xdc/0xe8
> >   deferred_probe_work_func+0xe8/0x130
> >   process_one_work+0x2a4/0x698
> >   worker_thread+0x53c/0x708
> >   kthread+0x1b4/0x1c8
> >   ret_from_fork+0x10/0x20
> >
> > Freed by task 27:
> >   kasan_save_stack+0x3c/0x68
> >   kasan_set_track+0x2c/0x40
> >   kasan_save_free_info+0x38/0x60
> >   __kasan_slab_free+0x100/0x170
> >   slab_free_freelist_hook+0xcc/0x218
> >   __kmem_cache_free+0x158/0x210
> >   kfree+0x88/0x140
> >   __clk_register+0x9d0/0xfb8
> >   devm_clk_hw_register+0x70/0x108
> >   bcm2835_register_clock+0x284/0x358
> >   bcm2835_clk_probe+0x2c4/0x438
> >   platform_probe+0x98/0x110
> >   really_probe+0x1e4/0x3e8
> >   __driver_probe_device+0xc0/0x1a0
> >   driver_probe_device+0x110/0x1e8
> >   __device_attach_driver+0xf0/0x1a8
> >   bus_for_each_drv+0xf8/0x178
> >   __device_attach+0x120/0x240
> >   device_initial_probe+0x1c/0x30
> >   bus_probe_device+0xdc/0xe8
> >   deferred_probe_work_func+0xe8/0x130
> >   process_one_work+0x2a4/0x698
> >   worker_thread+0x53c/0x708
> >   kthread+0x1b4/0x1c8
> >   ret_from_fork+0x10/0x20
> >
> > The buggy address belongs to the object at ffffff8043fd0800
> >   which belongs to the cache kmalloc-512 of size 512
> > The buggy address is located 384 bytes inside of
> >   freed 512-byte region [ffffff8043fd0800, ffffff8043fd0a00)
> >
> > The buggy address belongs to the physical page:
> > page:fffffffe010ff400 refcount:1 mapcount:0 mapping:0000000000000000 index:0xffffff8043fd0e00 pfn:0x43fd0
> > head:fffffffe010ff400 order:3 entire_mapcount:0 nr_pages_mapped:0 pincount:0
> > flags: 0x4000000000000840(slab|head|zone=1)
> > page_type: 0xffffffff()
> > raw: 4000000000000840 ffffff8040002f40 ffffff8040000a50 ffffff8040000a50
> > raw: ffffff8043fd0e00 0000000000150002 00000001ffffffff 0000000000000000
> > page dumped because: kasan: bad access detected
> >
> > Memory state around the buggy address:
> >   ffffff8043fd0880: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> >   ffffff8043fd0900: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > > ffffff8043fd0980: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> >                     ^
> >   ffffff8043fd0a00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> >   ffffff8043fd0a80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> > ==================================================================
> >
> > Fixes: 9d05ae531c2c ("clk: Initialize struct clk_core kref earlier")
> > Signed-off-by: Yun Zhou <yun.zhou@xxxxxxxxxxxxx>
> Is that the correct Fixes tag? What do you think about this instead?
>
> Fixes: fc0c209c147f ("clk: Allow parents to be specified without string names")
>
> Otherwise, the patch itself looks good to me.
>
> Brian