[PATCH 0/4] remoteproc: qcom: Fix DA-to-VA bounds checks

[PATCH 0/4] remoteproc: qcom: Fix DA-to-VA bounds checks

[Atul Akella]

This series fixes the same bounds check bug in da_to_va() across four
Qualcomm remoteproc PIL drivers: qcom_q6v5_adsp, qcom_q6v5_pas,
qcom_q6v5_wcss, and qcom_wcnss.

In each driver, offset is a signed 32bit integer assigned from
the 64 bit difference da - <base>, where both operands are u64.
If da is smaller than <base>, the subtraction underflows before
the result is truncated on assignment. This can produce a small
positive offset and bypass the existing underflow check.
Separately, the offset + len bounds check can overflow.

da and len originate from firmware ELF program headers
(p_paddr and p_memsz) via rproc_da_to_va(). The returned
address is used by the ELF loader to write firmware segments, so a
malformed firmware image can cause writes outside the carveout.

Each patch keeps offset in u64, rejects addresses below the
relocation base before subtraction, and uses a subtraction based
bounds check to avoid overflow.

Atul Akella (4):
  remoteproc: qcom: Fix bounds check in adsp_da_to_va()
  remoteproc: qcom: Fix bounds check in qcom_pas_da_to_va()
  remoteproc: qcom: Fix bounds check in q6v5_wcss_da_to_va()
  remoteproc: qcom: Fix bounds check in wcnss_da_to_va()

 drivers/remoteproc/qcom_q6v5_adsp.c | 6 ++++--
 drivers/remoteproc/qcom_q6v5_pas.c  | 6 ++++--
 drivers/remoteproc/qcom_q6v5_wcss.c | 6 ++++--
 drivers/remoteproc/qcom_wcnss.c     | 6 ++++--
 4 files changed, 16 insertions(+), 8 deletions(-)

-- 
2.43.0

[PATCH 1/4] remoteproc: qcom: Fix bounds check in adsp_da_to_va()

[Atul Akella]

offset is calculated as da - adsp->mem_reloc and stored in a
32 bit signed int, while da and adsp->mem_reloc are both u64.
For a sufficiently small da, the 64bit difference is truncated
into the 32 bit int, bypassing the offset < 0 underflow check
with a small positive value. Additionally, offset + len >
adsp->mem_size can itself overflow when offset and len are both
large, providing a second bypass.

da and len come from firmware's ELF program headers via
rproc_da_to_va(), and the result is used by the ELF loader
to write firmware segments, so a malformed firmware image can
cause writes outside the carveout.

Rewrite the bounds check entirely in u64 arithmetic. Reject
underflow before computing offset (da < adsp->mem_reloc).
Replace the addition based overflow check with a
subtraction based one (len > adsp->mem_size - offset) so no
intermediate sum can wrap.

Fixes: dc160e449122 ("remoteproc: qcom: Introduce Non-PAS ADSP PIL driver")
Cc: stable@vger.kernel.org
Signed-off-by: Atul Akella <atul.akella@gmail.com>
---
 drivers/remoteproc/qcom_q6v5_adsp.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c b/drivers/remoteproc/qcom_q6v5_adsp.c
index 46d9169a37f6..1bf87cd0e9dd 100644
--- a/drivers/remoteproc/qcom_q6v5_adsp.c
+++ b/drivers/remoteproc/qcom_q6v5_adsp.c
@@ -486,10 +486,12 @@ static int adsp_stop(struct rproc *rproc)
 static void *adsp_da_to_va(struct rproc *rproc, u64 da, size_t len, bool *is_iomem)
 {
 	struct qcom_adsp *adsp = rproc->priv;
-	int offset;
+	u64 offset;
 
+	if (da < adsp->mem_reloc)
+		return NULL;
 	offset = da - adsp->mem_reloc;
-	if (offset < 0 || offset + len > adsp->mem_size)
+	if (offset > adsp->mem_size || len > adsp->mem_size - offset)
 		return NULL;
 
 	return (__force void *)adsp->mem_region + offset;
-- 
2.43.0

[PATCH 2/4] remoteproc: qcom: Fix bounds check in qcom_pas_da_to_va()

[Atul Akella]

offset is calculated as da - pas->mem_reloc and stored in a
32 bit signed int, while da and pas->mem_reloc are both u64.
For a sufficiently small da, the 64bit difference is truncated
into the 32 bit int, bypassing the offset < 0 underflow check
with a small positive value. Additionally, offset + len >
pas->mem_size can itself overflow when offset and len are both
large, providing a second bypass.

da and len come from firmware's ELF program headers via
rproc_da_to_va(), and the result is used by the ELF loader
to write firmware segments, so a malformed firmware image can
cause writes outside the carveout.

Rewrite the bounds check entirely in u64 arithmetic. Reject
underflow before computing offset (da < pas->mem_reloc).
Replace the addition based overflow check with a
subtraction based one (len > pas->mem_size - offset) so no
intermediate sum can wrap.

Fixes: b9e718e950c3 ("remoteproc: Introduce Qualcomm ADSP PIL")
Cc: stable@vger.kernel.org
Signed-off-by: Atul Akella <atul.akella@gmail.com>
---
 drivers/remoteproc/qcom_q6v5_pas.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index f37b77ae187e..8cade27a6495 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -473,10 +473,12 @@ static int qcom_pas_stop(struct rproc *rproc)
 static void *qcom_pas_da_to_va(struct rproc *rproc, u64 da, size_t len, bool *is_iomem)
 {
 	struct qcom_pas *pas = rproc->priv;
-	int offset;
+	u64 offset;
 
+	if (da < pas->mem_reloc)
+		return NULL;
 	offset = da - pas->mem_reloc;
-	if (offset < 0 || offset + len > pas->mem_size)
+	if (offset > pas->mem_size || len > pas->mem_size - offset)
 		return NULL;
 
 	if (is_iomem)
-- 
2.43.0

[PATCH 3/4] remoteproc: qcom: Fix bounds check in q6v5_wcss_da_to_va()

[Atul Akella]

offset is calculated as da - wcss->mem_reloc and stored in a
32 bit signed int, while da and wcss->mem_reloc are both u64.
For a sufficiently small da, the 64bit difference is truncated
into the 32 bit int, bypassing the offset < 0 underflow check
with a small positive value. Additionally, offset + len >
wcss->mem_size can itself overflow when offset and len are both
large, providing a second bypass.

da and len come from firmware's ELF program headers via
rproc_da_to_va(), and the result is used by the ELF loader
to write firmware segments, so a malformed firmware image can
cause writes outside the carveout.

Rewrite the bounds check entirely in u64 arithmetic. Reject
underflow before computing offset (da < wcss->mem_reloc).
Replace the addition based overflow check with a
subtraction based one (len > wcss->mem_size - offset) so no
intermediate sum can wrap.

Fixes: 3a3d4163e0bf ("remoteproc: qcom: Introduce Hexagon V5 based WCSS driver")
Cc: stable@vger.kernel.org
Signed-off-by: Atul Akella <atul.akella@gmail.com>
---
 drivers/remoteproc/qcom_q6v5_wcss.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/remoteproc/qcom_q6v5_wcss.c b/drivers/remoteproc/qcom_q6v5_wcss.c
index b391724cfd08..e67dc3ff8a07 100644
--- a/drivers/remoteproc/qcom_q6v5_wcss.c
+++ b/drivers/remoteproc/qcom_q6v5_wcss.c
@@ -740,10 +740,12 @@ static int q6v5_wcss_stop(struct rproc *rproc)
 static void *q6v5_wcss_da_to_va(struct rproc *rproc, u64 da, size_t len, bool *is_iomem)
 {
 	struct q6v5_wcss *wcss = rproc->priv;
-	int offset;
+	u64 offset;
 
+	if (da < wcss->mem_reloc)
+		return NULL;
 	offset = da - wcss->mem_reloc;
-	if (offset < 0 || offset + len > wcss->mem_size)
+	if (offset > wcss->mem_size || len > wcss->mem_size - offset)
 		return NULL;
 
 	return wcss->mem_region + offset;
-- 
2.43.0

[PATCH 4/4] remoteproc: qcom: Fix bounds check in wcnss_da_to_va()

[Atul Akella]

offset is calculated as da - wcnss->mem_reloc and stored in a
32 bit signed int, while da and wcnss->mem_reloc are both u64.
For a sufficiently small da, the 64bit difference is truncated
into the 32 bit int, bypassing the offset < 0 underflow check
with a small positive value. Additionally, offset + len >
wcnss->mem_size can itself overflow when offset and len are both
large, providing a second bypass.

da and len come from firmware's ELF program headers via
rproc_da_to_va(), and the result is used by the ELF loader
to write firmware segments, so a malformed firmware image can
cause writes outside the carveout.

Rewrite the bounds check entirely in u64 arithmetic. Reject
underflow before computing offset (da < wcnss->mem_reloc).
Replace the addition based overflow check with a
subtraction based one (len > wcnss->mem_size - offset) so no
intermediate sum can wrap.

Fixes: aed361adca9f ("remoteproc: qcom: Introduce WCNSS peripheral image loader")
Cc: stable@vger.kernel.org
Signed-off-by: Atul Akella <atul.akella@gmail.com>
---
 drivers/remoteproc/qcom_wcnss.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/remoteproc/qcom_wcnss.c b/drivers/remoteproc/qcom_wcnss.c
index c856a92af43c..d95530e8be71 100644
--- a/drivers/remoteproc/qcom_wcnss.c
+++ b/drivers/remoteproc/qcom_wcnss.c
@@ -321,10 +321,12 @@ static int wcnss_stop(struct rproc *rproc)
 static void *wcnss_da_to_va(struct rproc *rproc, u64 da, size_t len, bool *is_iomem)
 {
 	struct qcom_wcnss *wcnss = rproc->priv;
-	int offset;
+	u64 offset;
 
+	if (da < wcnss->mem_reloc)
+		return NULL;
 	offset = da - wcnss->mem_reloc;
-	if (offset < 0 || offset + len > wcnss->mem_size)
+	if (offset > wcnss->mem_size || len > wcnss->mem_size - offset)
 		return NULL;
 
 	return (__force void *)wcnss->mem_region + offset;
-- 
2.43.0