linux-kernel - Re: [PATCH] remoteproc: core: Fix ida

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <Z03k7v6JuA2bCj9x@p14s>
Date: Mon, 2 Dec 2024 09:48:46 -0700
From: Mathieu Poirier <mathieu.poirier@...aro.org>
To: Arnaud Pouliquen <arnaud.pouliquen@...s.st.com>
Cc: Bjorn Andersson <andersson@...nel.org>,
	linux-remoteproc@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-stm32@...md-mailman.stormreply.com
Subject: Re: [PATCH] remoteproc: core: Fix ida_free call while not allocated

On Fri, Nov 22, 2024 at 06:51:27PM +0100, Arnaud Pouliquen wrote:
> In the rproc_alloc() function, on error, put_device(&rproc->dev) is
> called, leading to the call of the rproc_type_release() function.
> An error can occurs before ida_alloc is called.
> 
> In such case in rproc_type_release(), the condition (rproc->index >= 0) is
> true as rproc->index has been  initialized to 0.
> ida_free() is called reporting a warning:
> [    4.181906] WARNING: CPU: 1 PID: 24 at lib/idr.c:525 ida_free+0x100/0x164
> [    4.186378] stm32-display-dsi 5a000000.dsi: Fixed dependency cycle(s) with /soc/dsi@...00000/panel@0
> [    4.188854] ida_free called for id=0 which is not allocated.
> [    4.198256] mipi-dsi 5a000000.dsi.0: Fixed dependency cycle(s) with /soc/dsi@...00000
> [    4.203556] Modules linked in: panel_orisetech_otm8009a dw_mipi_dsi_stm(+) gpu_sched dw_mipi_dsi stm32_rproc stm32_crc32 stm32_ipcc(+) optee(+)
> [    4.224307] CPU: 1 UID: 0 PID: 24 Comm: kworker/u10:0 Not tainted 6.12.0 #442
> [    4.231481] Hardware name: STM32 (Device Tree Support)
> [    4.236627] Workqueue: events_unbound deferred_probe_work_func
> [    4.242504] Call trace:
> [    4.242522]  unwind_backtrace from show_stack+0x10/0x14
> [    4.250218]  show_stack from dump_stack_lvl+0x50/0x64
> [    4.255274]  dump_stack_lvl from __warn+0x80/0x12c
> [    4.260134]  __warn from warn_slowpath_fmt+0x114/0x188
> [    4.265199]  warn_slowpath_fmt from ida_free+0x100/0x164
> [    4.270565]  ida_free from rproc_type_release+0x38/0x60
> [    4.275832]  rproc_type_release from device_release+0x30/0xa0
> [    4.281601]  device_release from kobject_put+0xc4/0x294
> [    4.286762]  kobject_put from rproc_alloc.part.0+0x208/0x28c
> [    4.292430]  rproc_alloc.part.0 from devm_rproc_alloc+0x80/0xc4
> [    4.298393]  devm_rproc_alloc from stm32_rproc_probe+0xd0/0x844 [stm32_rproc]
> [    4.305575]  stm32_rproc_probe [stm32_rproc] from platform_probe+0x5c/0xbc
> 
> 
> Calling ida_alloc earlier in rproc_alloc ensures that the rproc->index is
> properly set.
> 
> Fixes: 08333b911f01 ("remoteproc: Directly use ida_alloc()/free()")
> 
> Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@...s.st.com>
> ---
> Note for backporting to previous kernel versions: The SHA 08333b911f01
> seems to correspond to the last commit that updated IDA allocation.
> The issue existed before, but the fix could not be applied without some
> rework.
> ---
>  drivers/remoteproc/remoteproc_core.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index f276956f2c5c..ef6febe35633 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -2486,6 +2486,13 @@ struct rproc *rproc_alloc(struct device *dev, const char *name,
>  	rproc->dev.driver_data = rproc;
>  	idr_init(&rproc->notifyids);
>  
> +	/* Assign a unique device index and name */
> +	rproc->index = ida_alloc(&rproc_dev_index, GFP_KERNEL);
> +	if (rproc->index < 0) {
> +		dev_err(dev, "ida_alloc failed: %d\n", rproc->index);
> +		goto put_device;
> +	}
> +
>  	rproc->name = kstrdup_const(name, GFP_KERNEL);
>  	if (!rproc->name)
>  		goto put_device;
> @@ -2496,13 +2503,6 @@ struct rproc *rproc_alloc(struct device *dev, const char *name,
>  	if (rproc_alloc_ops(rproc, ops))
>  		goto put_device;
>  
> -	/* Assign a unique device index and name */
> -	rproc->index = ida_alloc(&rproc_dev_index, GFP_KERNEL);
> -	if (rproc->index < 0) {
> -		dev_err(dev, "ida_alloc failed: %d\n", rproc->index);
> -		goto put_device;
> -	}
> -

I have applied this patch.

Thanks,
Mathieu

>  	dev_set_name(&rproc->dev, "remoteproc%d", rproc->index);
>  
>  	atomic_set(&rproc->power, 0);
> 
> base-commit: adc218676eef25575469234709c2d87185ca223a
> -- 
> 2.25.1
>