On Thu, Aug 27, 2026 at 02:31:36PM +0200, Marcel Hofmann wrote: > From: Marcel Hofmann <[email protected]> > > 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 <[email protected]> > --- > > 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/[email protected]/ > > 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 >
