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

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

[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.

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 <rosenp@gmail.com>
---
 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(-)

diff --git a/drivers/dma/bestcomm/gen_bd.c b/drivers/dma/bestcomm/gen_bd.c
index 61b5746e1a97..7bbe9cd7b39c 100644
--- a/drivers/dma/bestcomm/gen_bd.c
+++ b/drivers/dma/bestcomm/gen_bd.c
@@ -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

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

[sashiko-bot]

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 <rosenp@gmail.com>

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.

> diff --git a/drivers/dma/bestcomm/gen_bd.c b/drivers/dma/bestcomm/gen_bd.c
> index 8a24a5cbc2633..43e5bdd6937ec 100644
> --- a/drivers/dma/bestcomm/gen_bd.c
> +++ b/drivers/dma/bestcomm/gen_bd.c
[ ... ]
> @@ -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().

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915072608.57887-1-rosenp@gmail.com?part=1

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

[Frank Li]

On Tue, Sep 15, 2026 at 12:26:08AM -0700, Rosen Penev wrote:
> 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 <rosenp@gmail.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  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(-)
>
> diff --git a/drivers/dma/bestcomm/gen_bd.c b/drivers/dma/bestcomm/gen_bd.c
> index 61b5746e1a97..7bbe9cd7b39c 100644
> --- a/drivers/dma/bestcomm/gen_bd.c
> +++ b/drivers/dma/bestcomm/gen_bd.c
> @@ -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
>

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

[Vinod Koul]

On Tue, 15 Sep 2026 00:26:08 -0700, Rosen Penev wrote:
> 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.
> 
> [...]

Applied, thanks!

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

Best regards,
-- 
~Vinod