dmaengine: bestcomm: gen_bd: fix out-of-bounds access in PSC parameter lookup

PATCH Applied Rosen Penev 4 participants 4 messages Sep 15 dmaengine

Share & tools

Rosen Penev

author

2026-09-15 07:26 UTC #0 lore raw

PATCH v4 1/1 dmaengine: bestcomm: gen_bd: fix out-of-bounds access in PSC parameter lookup

The bcom_psc_params[] array has 6 entries (indices 0-5), but bcom_psc_gen_bd_rx_init() checked against MPC52xx_PSC_MAXNUM which can be 12 when CONFIG_PPC_MPC512x is set, allowing indices 6-11 to pass and read past the array. The tx init function had no bounds check at all.

A malformed device tree with a large cell-index could therefore trigger an out-of-bounds read. The garbage initiator and ipr values would then be used for MMIO writes via out_8(&bcom_eng->regs->ipr[...], ...), potentially causing out-of-bounds MMIO accesses.

Assisted-by LLM

Signed-off-by Rosen Penev <[email protected]>

---
v4: actually bcom_psc_gen_bd_tx_init is still needed.
v3: reduce bounds check to single function. It's not used anywhere anyway.
v2: fix title

 drivers/dma/bestcomm/gen_bd.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

drivers/dma/bestcomm/gen_bd.c +4 -1

index 61b5746e1a97..7bbe9cd7b39c 100644
@@ -321,7 +321,7 @@ static const struct bcom_psc_params bcom_psc_params[] = {
 struct bcom_task * bcom_psc_gen_bd_rx_init(unsigned psc_num, int queue_len,
 					   phys_addr_t fifo, int maxbufsize)
 {
-	if (psc_num >= MPC52xx_PSC_MAXNUM)
+	if (psc_num >= ARRAY_SIZE(bcom_psc_params))
 		return NULL;
 
 	return bcom_gen_bd_rx_init(queue_len, fifo,

@@ -342,6 +342,9 @@ EXPORT_SYMBOL_GPL(bcom_psc_gen_bd_rx_init);
 struct bcom_task *
 bcom_psc_gen_bd_tx_init(unsigned psc_num, int queue_len, phys_addr_t fifo)
 {
+	if (psc_num >= ARRAY_SIZE(bcom_psc_params))
+		return NULL;
+
 	struct psc;
 	return bcom_gen_bd_tx_init(queue_len, fifo,
 				   bcom_psc_params[psc_num].tx_initiator,

Signature · 1 line

2.55.0

[email protected]

AI review

2026-09-15 07:37 UTC #1 lore raw

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] dmaengine: bestcomm: useless struct declaration mixed with code --

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] dmaengine: bestcomm: useless struct declaration mixed with code
--

commit b34f7960c5a14d5c07b8bccee4d8da1a3d8c6985
Author: Rosen Penev <[email protected]>

dmaengine: bestcomm: gen_bd: fix out-of-bounds access in PSC parameter lookup

Quoted 4 lines

drivers/dma/bestcomm/gen_bd.c +0 -0
index 8a24a5cbc2633..43e5bdd6937ec 100644

[ ... ]

drivers/dma/bestcomm/gen_bd.c EXPORT_SYMBOL_GPL(bcom_psc_gen_bd_rx_init); · line 342

Quoted 8 lines

drivers/dma/bestcomm/gen_bd.c +3 -0

@@ -336,6 +336,9 @@ EXPORT_SYMBOL_GPL(bcom_psc_gen_bd_rx_init);
 struct bcom_task *
 bcom_psc_gen_bd_tx_init(unsigned psc_num, int queue_len, phys_addr_t fifo)
 {
+	if (psc_num >= ARRAY_SIZE(bcom_psc_params))
+		return NULL;
+
 	struct psc;

[Severity: Low]
Is this struct psc; declaration needed here? It appears to be a useless forward declaration that is now awkwardly positioned after an active statement in bcom_psc_gen_bd_tx_init().

Quoted 2 lines

drivers/dma/bestcomm/gen_bd.c +0 -0

 	return bcom_gen_bd_tx_init(queue_len, fifo,
 				   bcom_psc_params[psc_num].tx_initiator,

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

Frank Li

Reviewed-by on v4 1/1

2026-09-15 14:39 UTC #2 lore raw

In reply to Rosen Penev

Quoted 6 lines · Rosen Penev

The bcom_psc_params[] array has 6 entries (indices 0-5), but bcom_psc_gen_bd_rx_init() checked against MPC52xx_PSC_MAXNUM which can be 12 when CONFIG_PPC_MPC512x is set, allowing indices 6-11 to pass and read past the array. The tx init function had no bounds check at all.

Quoted 8 lines

A malformed device tree with a large cell-index could therefore trigger an out-of-bounds read. The garbage initiator and ipr values would then be used for MMIO writes via out_8(&bcom_eng->regs->ipr[...], ...), potentially causing out-of-bounds MMIO accesses.
Assisted-by LLM
Signed-off-by Rosen Penev <[email protected]>
---

Reviewed-by Frank Li <[email protected]>

Quoted 32 lines

v4: actually bcom_psc_gen_bd_tx_init is still needed.
v3: reduce bounds check to single function. It's not used anywhere anyway.
v2: fix title

 drivers/dma/bestcomm/gen_bd.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

drivers/dma/bestcomm/gen_bd.c +4 -2

index 61b5746e1a97..7bbe9cd7b39c 100644
@@ -321,7 +321,7 @@ static const struct bcom_psc_params bcom_psc_params[] = {
 struct bcom_task * bcom_psc_gen_bd_rx_init(unsigned psc_num, int queue_len,
 					   phys_addr_t fifo, int maxbufsize)
 {
-	if (psc_num >= MPC52xx_PSC_MAXNUM)
+	if (psc_num >= ARRAY_SIZE(bcom_psc_params))
 		return NULL;
 
 	return bcom_gen_bd_rx_init(queue_len, fifo,

@@ -342,6 +342,9 @@ EXPORT_SYMBOL_GPL(bcom_psc_gen_bd_rx_init);
 struct bcom_task *
 bcom_psc_gen_bd_tx_init(unsigned psc_num, int queue_len, phys_addr_t fifo)
 {
+	if (psc_num >= ARRAY_SIZE(bcom_psc_params))
+		return NULL;
+
 	struct psc;
 	return bcom_gen_bd_tx_init(queue_len, fifo,
 				   bcom_psc_params[psc_num].tx_initiator,
--

2.55.0

Vinod Koul

maintainer Applied

2026-09-15 17:36 UTC #3 lore raw

In reply to Rosen Penev

Quoted 6 lines · Rosen Penev

The bcom_psc_params[] array has 6 entries (indices 0-5), but bcom_psc_gen_bd_rx_init() checked against MPC52xx_PSC_MAXNUM which can be 12 when CONFIG_PPC_MPC512x is set, allowing indices 6-11 to pass and read past the array. The tx init function had no bounds check at all.

Quoted 6 lines

A malformed device tree with a large cell-index could therefore trigger an out-of-bounds read. The garbage initiator and ipr values would then be used for MMIO writes via out_8(&bcom_eng->regs->ipr[...], ...), potentially causing out-of-bounds MMIO accesses.
[...]

Applied, thanks!

[1/1] dmaengine: bestcomm: gen_bd: fix out-of-bounds access in PSC parameter lookup

      commit: a8d1f582fae8231bd0cf66164ef140cd410382d4

Best regards,

Signature · 1 line

~Vinod