Re: [PATCH v2 3/5] remoteproc: mark wc-ioremap carveouts as iomem

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Critical] Integer Overflow in `rproc_elf_load_segments` allowing Out-of-Bounds Read.
- [Critical] Integer Overflow in `rproc_da_to_va` bounds check allowing Out-of-Bounds Write.
- [High] The patch correctly fixes the generic `rproc_mem_entry_ioremap_wc()` helper but overlooks driver-specific allocators and `da_to_va` callbacks that exhibit the exact same bug.
- [High] Debugfs interface `rproc_trace_read` performs standard memory accesses on I/O memory, triggering panics.
--

commit 185669571331faa11cc44eacd2568b0e202bcf9d
Author: Ben Levinsky <ben.levinsky@amd.com>

remoteproc: mark wc-ioremap carveouts as iomem

This commit marks shared wc-ioremap carveouts as iomem in the generic
rproc_mem_entry_ioremap_wc() helper. This ensures the remoteproc ELF loader
uses the proper I/O memory accessors instead of falling back to standard
memcpy() and memset().

> diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h
> index f5b34aabed5bb..9955e512f073d 100644
> --- a/drivers/remoteproc/remoteproc_internal.h
> +++ b/drivers/remoteproc/remoteproc_internal.h
> @@ -136,6 +136,7 @@ static inline int rproc_mem_entry_ioremap_wc(struct rproc *rproc,
>  	}
>  
>  	mem->va = (__force void *)va;
> +	mem->is_iomem = true;

Does this change also need to be applied to driver-specific allocators and
da_to_va callbacks?

For example, in drivers/remoteproc/xlnx_r5_remoteproc.c, tcm_mem_map() maps
memory via ioremap_wc() but does not set is_iomem to true.

Similarly, keystone_rproc_da_to_va() and omap_rproc_da_to_va() appear to
ignore the is_iomem pointer despite returning devm_ioremap_resource mapped
memory. In imx_rproc_da_to_sys(), is_iomem is left false for regions mapped
via devm_ioremap_wc() when ATT_IOMEM is not set.

>  
>  	return 0;
>  }

Are there potential integer overflows in rproc_elf_load_segments() when
parsing ELF headers?

If offset and filesz are parsed directly from 64-bit ELF program headers,
could their sum wrap around:

rproc_elf_load_segments() {
    ...
    if (offset + filesz > fw->size)
    ...
        memcpy(ptr, elf_data + offset, filesz);
    ...
}

If offset is near the maximum 64-bit value and filesz is small, the bounds
check could be bypassed, resulting in an out-of-bounds read.

Is there a similar overflow risk in the bounds check within rproc_da_to_va()?

rproc_da_to_va() {
    ...
    if (offset + len > carveout->len)
    ...
}

If len is a size_t and offset is an int, could an ELF segment with a memsz
near the maximum size_t value cause the sum to wrap around and bypass the
bounds check?

This could affect rproc_elf_load_segments() if it executes a memset using
the large memsz value, resulting in an out-of-bounds write.

Does the debugfs interface have a related regression with I/O memory access?

In rproc_trace_read(), the memory type is ignored by passing NULL for the
is_iomem flag to rproc_da_to_va():

rproc_trace_read() {
    ...
    va = rproc_da_to_va(rproc, trace->da, trace->len, NULL);
    ...
}

If the returned virtual address is mapped as device memory, such as via
devm_ioremap_resource() in keystone_remoteproc.c, could passing it directly
to strnlen() and simple_read_from_buffer() trigger alignment faults on
strict architectures?

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