remoteproc: mtk_scp: Fix bounds checks in da_to_va callbacks

mt8183_scp_da_to_va() and mt8192_scp_da_to_va() validate the device address and length that the ELF loader passes in from the firmware's program headers. Both checks can be bypassed through integer truncation or wraparound. These two patches fix them with overflow-safe comparisons.

The fixes have different origin commits, so they are split for stable backporting.

Compile tested only (arm64 defconfig, W=1). Not tested on hardware.

Atul Akella (2):
remoteproc: mtk_scp: Fix bounds check in mt8183_scp_da_to_va()
remoteproc: mtk_scp: Fix bounds check in mt8192_scp_da_to_va()

 drivers/remoteproc/mtk_scp.c | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

2.43.0

mt8183_scp_da_to_va() translates a device address to a pointer into SCP SRAM or DRAM. Its bounds check is wrong in two ways:

- offset is an int but is assigned from a u64. For the DRAM window, da = dma_addr + 0x100000010 is truncated to offset 0x10 and passes the check. There is also no da >= dma_addr check, so a da below dma_addr wraps in the u64 subtraction and can truncate to a small positive offset.

- (offset + len) is computed in size_t and wraps for a large len. With offset = 0x100 and len = (size_t)-0x80, the sum is 0x80 and the check passes.

da and len come from the firmware's ELF program headers (p_paddr/p_memsz) via rproc_da_to_va(), and the returned pointer is then written to by the ELF loader, so a malformed firmware image can make the loader write outside the intended region.

Use a u64 offset, check da >= dma_addr before subtracting, and compare len against the remaining window size instead of adding it to offset.

---
 drivers/remoteproc/mtk_scp.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

index 9751acc2bf83..d18ae676260a 100644
@@ -754,17 +754,19 @@ static int scp_start(struct rproc *rproc)
 
 static void *mt8183_scp_da_to_va(struct mtk_scp *scp, u64 da, size_t len)
 {
-	int offset;
+	u64 offset;
 	const struct mtk_scp_sizes_data *scp_sizes;
 
 	scp_sizes = scp->data->scp_sizes;
 	if (da < scp->sram_size) {
 		offset = da;
-		if (offset >= 0 && (offset + len) <= scp->sram_size)
+		if (len <= scp->sram_size - offset)
 			return (void __force *)scp->sram_base + offset;
 	} else if (scp_sizes->max_dram_size) {
 		offset = da - scp->dma_addr;
-		if (offset >= 0 && (offset + len) <= scp_sizes->max_dram_size)
+		if (da >= scp->dma_addr &&
+		    offset <= scp_sizes->max_dram_size &&
+		    len <= scp_sizes->max_dram_size - offset)
 			return scp->cpu_addr + offset;
 	}
 

2.43.0

mt8192_scp_da_to_va() translates a device address to a pointer into SCP SRAM, L1TCM or DRAM. Its bounds checks have two problems:

- (da + len) is computed in u64 and wraps for a large da or len. With da = 0xfffffffffffffff0 and len = 0x20, the sum is 0x10 and passes "<= base + size" while da >= base also holds.

- offset is an int but is assigned from a u64 difference, so it is truncated before being added to the region's base pointer.

da and len come from the firmware's ELF program headers (p_paddr/p_memsz) via rproc_da_to_va(), and the returned pointer is then written to by the ELF loader, so a malformed firmware image can make the loader write outside the intended region.

Use a u64 offset and compare len against the remaining region size instead of adding it to da. Apply this to the SRAM, L1TCM and DRAM regions.

---
 drivers/remoteproc/mtk_scp.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

index d18ae676260a..b1d83265fc26 100644
@@ -775,30 +775,30 @@ static void *mt8183_scp_da_to_va(struct mtk_scp *scp, u64 da, size_t len)
 
 static void *mt8192_scp_da_to_va(struct mtk_scp *scp, u64 da, size_t len)
 {
-	int offset;
+	u64 offset;
 	const struct mtk_scp_sizes_data *scp_sizes;
 
 	scp_sizes = scp->data->scp_sizes;
-	if (da >= scp->sram_phys &&
-	    (da + len) <= scp->sram_phys + scp->sram_size) {
+	if (da >= scp->sram_phys) {
 		offset = da - scp->sram_phys;
-		return (void __force *)scp->sram_base + offset;
+		if (offset <= scp->sram_size && len <= scp->sram_size - offset)
+			return (void __force *)scp->sram_base + offset;
 	}
 
 	/* optional memory region */
-	if (scp->cluster->l1tcm_size &&
-	    da >= scp->cluster->l1tcm_phys &&
-	    (da + len) <= scp->cluster->l1tcm_phys + scp->cluster->l1tcm_size) {
+	if (scp->cluster->l1tcm_size && da >= scp->cluster->l1tcm_phys) {
 		offset = da - scp->cluster->l1tcm_phys;
-		return (void __force *)scp->cluster->l1tcm_base + offset;
+		if (offset <= scp->cluster->l1tcm_size &&
+		    len <= scp->cluster->l1tcm_size - offset)
+			return (void __force *)scp->cluster->l1tcm_base + offset;
 	}
 
 	/* optional memory region */
-	if (scp_sizes->max_dram_size &&
-	    da >= scp->dma_addr &&
-	    (da + len) <= scp->dma_addr + scp_sizes->max_dram_size) {
+	if (scp_sizes->max_dram_size && da >= scp->dma_addr) {
 		offset = da - scp->dma_addr;
-		return scp->cpu_addr + offset;
+		if (offset <= scp_sizes->max_dram_size &&
+		    len <= scp_sizes->max_dram_size - offset)
+			return scp->cpu_addr + offset;
 	}
 
 	return NULL;

2.43.0

Hi,

Just following up on this series. Both patches are compile-tested only (arm64 defconfig, W=1).
I haven't tested them on actual mt8183/mt8192 hardware, but I'd be happy to help with hardware testing if someone has access to a suitable board. I'm also open to addressing any review feedback.

Thanks,
Atul

That patchset did not show up on my radar because it did not include the people listed by "get_maintainer.pl". Please resend with the correct recipient list.

I just noticed that individual patches had the right recipients but not the cover letter. Next time, please use the same recipient list for the cover letter. I will add your work to my review queue.

I will apply this set when 7.4-rc1 comes out.

Thanks,
Mathieu

Applied.

Thanks,
Mathieu