Message-ID: <a77dc81a-e8fa-4f9e-8915-3b8f352c114c@windriver.com>
Date: Tue, 30 Sep 2025 12:59:14 +0800
From: "Zhou, Yun" <yun.zhou@...driver.com>
To: Brian Masney <bmasney@...hat.com>
Cc: mturquette@...libre.com, sboyd@...nel.org, dianders@...omium.org,
        linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] clk: fix slab-use-after-free when
 clk_core_populate_parent_map failed

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@...driver.com>
> 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
>

Powered by blists - more mailing lists