Re: [PATCH] spi: imx: fix use-after-free during driver removal

[Frank Li <Frank.li@xxxxxxx>]

On Thu, Aug 08, 2024 at 02:38:45PM +0200, Kirill Yatsenko wrote:
> With the CONFIG_SLUB_DEBUG_ON enabled the unhandled fault error appears
> when unbinding the driver.
>
> The spi controller driver memory is freed inside the spi_imx_remove prior
> to executing PM callbacks thus leading to use-after-free.
>
> Fix it by switching to the devm version of spi_register_controller.
>
> Alignment trap: not handling instruction e1932f9f at [<80632434>]
> 8<--- cut here ---
> Unhandled fault: alignment exception (0x001) at 0x6b6b6c53
> [6b6b6c53] *pgd=00000000
> Internal error: : 1 [#1] PREEMPT SMP ARM
> Modules linked in: cdc_mbim cdc_wdm cdc_ncm cdc_ether usbnet cdc_acm usb_serial_simple usbserial usb_f_rndis u_ether wl18xx wlcore mac80211 libarc4 cfg80211 wlcore_sdio phy_generic ci_hdrc_imx ci_hdrc ulpi usbmisc_imx roles pwm_imx27 pwm_beeper evbug libcomposite udc_core configfs nfnetlink
> CPU: 2 PID: 1241 Comm: rebind.sh Not tainted 6.10.0-dnm3pv2-dnm3pv2-ga03695deba11 #1
> Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
> PC is at __pm_runtime_resume+0x58/0x6c
> LR is at spi_imx_remove+0x1c/0xa8
> pc : [<80632438>]    lr : [<806ebefc>]    psr: 20010013
> sp : f1d81e88  ip : 83c0e204  fp : 00000000
> r10: 00000000  r9 : 00000000  r8 : 82dd9454
> r7 : 82dda054  r6 : 810f82f0  r5 : 00000004  r4 : 6b6b6b6b
> r3 : 6b6b6c53  r2 : 85321240  r1 : 00000004  r0 : 6b6b6b6b
> Flags: nzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
> Control: 10c5387d  Table: 1687c04a  DAC: 00000051
> Register r0 information: non-paged memory
> Register r1 information: non-paged memory
> Register r2 information: slab task_struct start 85321200 data offset 64 pointer offset 0 size 2176 allocated at copy_process+0x1d8/0x1b2c
>     kmem_cache_alloc_node_noprof+0x10c/0x334
>     copy_process+0x1d8/0x1b2c
>     kernel_clone+0xa4/0x340
>     sys_clone+0x70/0x94
>     ret_fast_syscall+0x0/0x54
>  Free path:
>     rcu_core+0x2b4/0x7e0
>     handle_softirqs+0xf4/0x25c
>     irq_exit+0x68/0x8c
>     call_with_stack+0x18/0x20
>     __irq_svc+0x98/0xc8
>     cpuidle_enter_state+0x168/0x37c
>     cpuidle_enter+0x40/0x50
>     do_idle+0x108/0x200
>     cpu_startup_entry+0x28/0x2c
>     secondary_start_kernel+0x12c/0x14c
>     0x10101340
> Register r3 information: non-paged memory
> Register r4 information: non-paged memory
> Register r5 information: non-paged memory
> Register r6 information: non-slab/vmalloc memory
> Register r7 information: slab kmalloc-512 start 82dd9e00 data offset 512 pointer offset 84 size 512 allocated at platform_device_alloc+0x20/0xb8
>     __kmalloc_noprof+0x148/0x380
>     platform_device_alloc+0x20/0xb8
>     of_device_alloc+0x34/0x178
>     of_platform_device_create_pdata+0x60/0x11c
>     of_platform_bus_create+0x1cc/0x35c
>     of_platform_bus_create+0x230/0x35c
>     of_platform_bus_create+0x230/0x35c
>     of_platform_bus_create+0x230/0x35c
>     of_platform_populate+0x80/0x110
>     imx6q_init_machine+0x98/0x21c
>     customize_machine+0x20/0x30
>     do_one_initcall+0x58/0x240
>     kernel_init_freeable+0x198/0x1f4
>     kernel_init+0x1c/0x12c
>     ret_from_fork+0x14/0x28
> Register r8 information: slab kmalloc-512 start 82dd9200 data offset 512 pointer offset 84 size 512 allocated at platform_device_alloc+0x20/0xb8
>     __kmalloc_noprof+0x148/0x380
>     platform_device_alloc+0x20/0xb8
>     of_device_alloc+0x34/0x178
>     of_platform_device_create_pdata+0x60/0x11c
>     of_platform_bus_create+0x1cc/0x35c
>     of_platform_bus_create+0x230/0x35c
>     of_platform_bus_create+0x230/0x35c
>     of_platform_populate+0x80/0x110
>     imx6q_init_machine+0x98/0x21c
>     customize_machine+0x20/0x30
>     do_one_initcall+0x58/0x240
>     kernel_init_freeable+0x198/0x1f4
>     kernel_init+0x1c/0x12c
>     ret_from_fork+0x14/0x28
> Register r9 information: NULL pointer
> Register r10 information: NULL pointer
> Register r11 information: NULL pointer
> Register r12 information: slab kmalloc-64 start 83c0e180 data offset 64 pointer offset 68 size 64 allocated at kobject_set_name_vargs+0x2c/0xa0
>     kmalloc_node_track_caller_noprof+0x14c/0x37c
>     kvasprintf+0x5c/0xcc
>     kobject_set_name_vargs+0x2c/0xa0
>     dev_set_name+0x2c/0x58
>     spi_register_controller+0xcc/0xc48
>     spi_imx_probe+0x41c/0x694
>     platform_probe+0x5c/0xb0
>     really_probe+0xe0/0x3cc
>     __driver_probe_device+0x9c/0x1e0
>     driver_probe_device+0x30/0xc0
>     __driver_attach+0x11c/0x1cc
>     bus_for_each_dev+0x7c/0xcc
>     bus_add_driver+0xe0/0x220
>     driver_register+0x7c/0x114
>     do_one_initcall+0x58/0x240
>     kernel_init_freeable+0x198/0x1f4
>  Free path:
>     kobject_put+0xd0/0x29c
>     spi_imx_remove+0x10/0xa8
>     platform_remove+0x20/0x5c
>     device_release_driver_internal+0x184/0x1f0
>     unbind_store+0x54/0x90
>     kernfs_fop_write_iter+0xfc/0x1e8
>     vfs_write+0x25c/0x450
>     ksys_write+0x70/0xf0
>     ret_fast_syscall+0x0/0x54
> Process rebind.sh (pid: 1241, stack limit = 0xf1d80000)
> Stack: (0xf1d81e88 to 0xf1d82000)
> 1e80:                   83c54f40 82dd9410 810f82f0 806ebefc 82dda010 8062698c
> 1ea0: 82dda010 80625014 810f3b60 82dda010 0000000c 810f82f0 f1d81f28 806228c4
> 1ec0: 85cad640 0000000c 85e56080 85e56090 f1d81f28 802dafe0 00000000 00000000
> 1ee0: 85108e00 802daee4 f1d81f80 0086ae30 85321240 80b14188 00000000 80257d8c
> 1f00: 8687c020 00000000 00000000 00000000 00010000 0000000c 0086ae30 00000000
> 1f20: 00000001 00000000 85108e00 00000000 00000000 00000000 00000000 00000000
> 1f40: 00000000 00000000 00000000 00000000 f1d81f50 16506a32 0000000c 85108e00
> 1f60: 85108e00 00000000 00000000 801002c4 85321240 00000004 00000000 802580e4
> 1f80: 00000000 00000000 0086b000 16506a32 0000002d 0000000c 0086ae30 76ef6ba8
> 1fa0: 00000004 80100060 0000000c 0086ae30 00000001 0086ae30 0000000c 00000001
> 1fc0: 0000000c 0086ae30 76ef6ba8 00000004 00000000 005a6dd0 00000000 00000000
> 1fe0: 00000004 7eef2468 76e982fb 76e155a6 40010030 00000001 00000000 00000000
> Call trace:
>  __pm_runtime_resume from spi_imx_remove+0x1c/0xa8
>  spi_imx_remove from platform_remove+0x20/0x5c
>  platform_remove from device_release_driver_internal+0x184/0x1f0
>  device_release_driver_internal from unbind_store+0x54/0x90
>  unbind_store from kernfs_fop_write_iter+0xfc/0x1e8
>  kernfs_fop_write_iter from vfs_write+0x25c/0x450
>  vfs_write from ksys_write+0x70/0xf0
>  ksys_write from ret_fast_syscall+0x0/0x54
> Exception stack(0xf1d81fa8 to 0xf1d81ff0)
> 1fa0:                   0000000c 0086ae30 00000001 0086ae30 0000000c 00000001
> 1fc0: 0000000c 0086ae30 76ef6ba8 00000004 00000000 005a6dd0 00000000 00000000
> 1fe0: 00000004 7eef2468 76e982fb 76e155a6
> Code: e8bd8070 e28030e8 f593f000 e1932f9f (e2822001)
> ---[ end trace 0000000000000000 ]---


Can you cut log short, which only show related part?

Frank

>
> Fixes: 307c897db762 ("spi: spi-imx: replace struct spi_imx_data::bitbang by pointer to struct spi_controller")
> Signed-off-by: Kirill Yatsenko <kirill.yatsenko@xxxxxxxxxxxxxxx>
> ---
>  drivers/spi/spi-imx.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
> index 4a56a5b16e12..14834c4e839a 100644
> --- a/drivers/spi/spi-imx.c
> +++ b/drivers/spi/spi-imx.c
> @@ -1854,7 +1854,7 @@ static int spi_imx_probe(struct platform_device *pdev)
>  	spi_imx->devtype_data->intctrl(spi_imx, 0);
>
>  	controller->dev.of_node = pdev->dev.of_node;
> -	ret = spi_register_controller(controller);
> +	ret = devm_spi_register_controller(&pdev->dev, controller);
>  	if (ret) {
>  		dev_err_probe(&pdev->dev, ret, "register controller failed\n");
>  		goto out_register_controller;
> @@ -1900,8 +1900,6 @@ static void spi_imx_remove(struct platform_device *pdev)
>  	struct spi_imx_data *spi_imx = spi_controller_get_devdata(controller);
>  	int ret;
>
> -	spi_unregister_controller(controller);
> -
>  	ret = pm_runtime_get_sync(spi_imx->dev);
>  	if (ret >= 0)
>  		writel(0, spi_imx->base + MXC_CSPICTRL);
> --
> 2.34.1
>