Re: [PATCH v2] remoteproc: imx_rproc: allow mappings ending at region boundary

[Mathieu Poirier <mathieu.poirier@linaro.org>]

On Thu, Aug 27, 2026 at 02:31:36PM +0200, Marcel Hofmann wrote:
> From: Marcel Hofmann <marcel.hofmann@ingenics-digital.com>
> 
> The address range checks in imx_rproc_da_to_sys() and
> imx_rproc_da_to_va() use a strict comparison for the exclusive end
> address of the requested range.
> 
> As a result, a valid request that ends exactly at the end of an address
> translation or mapped memory region is rejected. For a region
> [start, start + size), a request [addr, addr + len) is contained when:
> 
>     addr >= start && addr + len <= start + size
> 
> This occurs when a loadable ELF segment fills an entire mapped memory
> region. This can be produced by a linker script that extends the
> resource table section to the end of its designated region:
> 
>     .resource_table :
>     {
>       . = ALIGN(8);
>       KEEP(*(.resource_table)) /* Resource table */
>       . = ALIGN(8);
>       . = ORIGIN(m_rsc_tbl) + LENGTH(m_rsc_tbl);
>     } > m_rsc_tbl =0x00
> 
> This produces a ELF program header like:
> 
>     LOAD  0x010000 0xa4220000 0xa4220000 0x01000 0x01000 R   0x1000
> 
> In this case, the segment size matches the mapped region size exactly,
> causing the address translation to fail with:
> 
>     bad phdr da 0xa4220000 mem 0x1000
> 
> Rework the upper-bound checks to allow ranges ending exactly at the region
> boundary while guarding against integer overflow. 
> 
> Fixes: a0ff4aa6f010 ("remoteproc: imx_rproc: add a NXP/Freescale imx_rproc driver")
> Signed-off-by: Marcel Hofmann <marcel.hofmann@ingenics-digital.com>
> ---
> 
> Changes in v2: 
> - Reworked the bounds check to guard against 64-bit integer overflow
> - Use u64 for offset instead of unsigned integer
> - calculate offset first and then validate len against remaining region
>   size
> 
> v1: https://lore.kernel.org/r/20260826155123.AEB731F000E9@smtp.kernel.org/
> 
>  drivers/remoteproc/imx_rproc.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> index 745ce52cd822..ea852ca143bb 100644
> --- a/drivers/remoteproc/imx_rproc.c
> +++ b/drivers/remoteproc/imx_rproc.c
> @@ -540,6 +540,7 @@ static int imx_rproc_da_to_sys(struct imx_rproc *priv, u64 da,
>  	/* parse address translation table */
>  	for (i = 0; i < dcfg->att_size; i++) {
>  		const struct imx_rproc_att *att = &dcfg->att[i];
> +		u64 offset;
>  
>  		/*
>  		 * Ignore entries not belong to current core:
> @@ -552,9 +553,11 @@ static int imx_rproc_da_to_sys(struct imx_rproc *priv, u64 da,
>  				continue;
>  		}
>  
> -		if (da >= att->da && da + len < att->da + att->size) {
> -			unsigned int offset = da - att->da;
> +		if (da < att->da)
> +			continue;
>  
> +		offset = da - att->da;
> +		if (offset <= att->size && len <= att->size - offset) {
>  			*sys = att->sa + offset;
>  			if (is_iomem)
>  				*is_iomem = att->flags & ATT_IOMEM;
> @@ -585,9 +588,14 @@ static void *imx_rproc_da_to_va(struct rproc *rproc, u64 da, size_t len, bool *i
>  		return NULL;
>  
>  	for (i = 0; i < IMX_RPROC_MEM_MAX; i++) {
> -		if (sys >= priv->mem[i].sys_addr && sys + len <
> -		    priv->mem[i].sys_addr +  priv->mem[i].size) {
> -			unsigned int offset = sys - priv->mem[i].sys_addr;
> +		u64 offset;
> +
> +		if (sys < priv->mem[i].sys_addr)
> +			continue;
> +
> +		offset = sys - priv->mem[i].sys_addr;
> +		if (offset <= priv->mem[i].size &&
> +		    len <= priv->mem[i].size - offset) {
>  			/* __force to make sparse happy with type conversion */
>  			va = (__force void *)(priv->mem[i].cpu_addr + offset);

This looks sensible.  That said, I would like to see someone on the NXP team
test this patch in different configuration.

Thanks,
Mathieu

>  			break;
> -- 
> 2.55.0
>