Re: [PATCH v2 2/5] remoteproc: switch exact-match drivers to wc-ioremap callbacks

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Format string vulnerability when initializing rproc_mem_entry using device tree node names
--

commit 1e23984cace57551a1444d0f4d0c49192a7ec852
Author: Ben Levinsky <ben.levinsky@amd.com>

remoteproc: switch exact-match drivers to wc-ioremap callbacks

This patch replaces the exact-match carveout map and unmap callbacks in
existing remoteproc drivers with the common wc-ioremap helpers.

> diff --git a/drivers/remoteproc/rcar_rproc.c b/drivers/remoteproc/rcar_rproc.c
> index 3c25625f966dc..e3121fadd2925 100644
> --- a/drivers/remoteproc/rcar_rproc.c
> +++ b/drivers/remoteproc/rcar_rproc.c
[ ... ]
> @@ -73,8 +44,8 @@ static int rcar_rproc_prepare(struct rproc *rproc)
>  		mem = rproc_mem_entry_init(dev, NULL,
>  					   res.start,
>  					   resource_size(&res), da,
> -					   rcar_rproc_mem_alloc,
> -					   rcar_rproc_mem_release,
> +					   rproc_mem_entry_ioremap_wc,
> +					   rproc_mem_entry_iounmap,
>  					   res.name);

This isn't a regression introduced by this patch, but since these lines are
being modified, could this cause a format string issue?

Looking at rproc_mem_entry_init(), it takes a format string as the name
parameter and passes it to vsnprintf(). If a dynamically loaded device
tree overlay has a node name in res.name containing format specifiers,
could it lead to arbitrary stack reads?

Would it be safer to use "%s", res.name here?

[ ... ]
> diff --git a/drivers/remoteproc/xlnx_r5_remoteproc.c b/drivers/remoteproc/xlnx_r5_remoteproc.c
> index 45a62cb980725..e5d1903c9636a 100644
> --- a/drivers/remoteproc/xlnx_r5_remoteproc.c
> +++ b/drivers/remoteproc/xlnx_r5_remoteproc.c
[ ... ]
> @@ -560,8 +521,8 @@ static int add_sram_carveouts(struct rproc *rproc)
>  		rproc_mem = rproc_mem_entry_init(&rproc->dev, NULL,
>  						 dma_addr,
>  						 len, da,
> -						 zynqmp_r5_mem_region_map,
> -						 zynqmp_r5_mem_region_unmap,
> +						 rproc_mem_entry_ioremap_wc,
> +						 rproc_mem_entry_iounmap,
>  						 sram->sram_res.name);

Similarly here, can passing sram->sram_res.name directly to
rproc_mem_entry_init() trigger a format string vulnerability if the node
name contains format specifiers?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260514162129.1504162-1-ben.levinsky@amd.com?part=2