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

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

Fix both functions to use ARRAY_SIZE(bcom_psc_params) for the bounds
check.

Also remove a stray unused 'struct psc;' forward declaration in
bcom_psc_gen_bd_tx_init.

Assisted-by: Opencode:big-pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/dma/bestcomm/gen_bd.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/bestcomm/gen_bd.c b/drivers/dma/bestcomm/gen_bd.c
index 61b5746e1a97..00dfdc4b8eba 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,7 +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)
 {
-	struct psc;
+	if (psc_num >= ARRAY_SIZE(bcom_psc_params))
+		return NULL;
+
 	return bcom_gen_bd_tx_init(queue_len, fifo,
 				   bcom_psc_params[psc_num].tx_initiator,
 				   bcom_psc_params[psc_num].tx_ipr);
-- 
2.54.0

Re: [PATCH] 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:

Pre-existing issues:
- [Critical] Unchecked array access in `mpc512x_psc_alloc_clock` and related clock functions where `psc_num` derived from device tree is used to index arrays, causing out-of-bounds reads and writes.
--

commit 7a3c433b5cd814a1fd0d2b57113cd41cb03279a5
Author: Rosen Penev <rosenp@gmail.com>

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

This commit fixes an out-of-bounds access issue in bcom_psc_gen_bd_rx_init()
and bcom_psc_gen_bd_tx_init() by properly using ARRAY_SIZE() for bounds
checking on psc_num. It also removes an unused forward declaration in
bcom_psc_gen_bd_tx_init().

> diff --git a/drivers/dma/bestcomm/gen_bd.c b/drivers/dma/bestcomm/gen_bd.c
> index 8a24a5cbc2633..75d1a6f543df9 100644
> --- a/drivers/dma/bestcomm/gen_bd.c
> +++ b/drivers/dma/bestcomm/gen_bd.c
> @@ -315,7 +315,7 @@ static struct 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))

[Severity: Critical]
This is a pre-existing issue, but while looking at this fix for the psc_num
bounds checking, I noticed a similar unchecked array access regression vector
using psc_num in drivers/tty/serial/mpc52xx_uart.c.

In mpc512x_psc_alloc_clock(), psc_num is calculated directly from
port->mapbase:

drivers/tty/serial/mpc52xx_uart.c:mpc512x_psc_alloc_clock() {
	...
	psc_num = (port->mapbase & 0xf00) >> 8;
	...
	psc_mclk_clk[psc_num] = clk;
	...
}

Because port->mapbase comes from the device tree via
of_address_to_resource(), a malformed device tree could supply an address
like 0x11F00, allowing psc_num to evaluate up to 15.

Since psc_mclk_clk and psc_ipg_clk are arrays sized to MPC52xx_PSC_MAXNUM
(which is 12 when CONFIG_PPC_MPC512x is defined), wouldn't this allow an
out-of-bounds write of a pointer to the BSS section?

The exact same unchecked psc_num calculation also appears in
mpc512x_psc_relse_clock() and mpc512x_psc_endis_clock(), leading to similar
out-of-bounds reads and writes.

>  		return NULL;
>  
>  	return bcom_gen_bd_rx_init(queue_len, fifo,
[ ... ]

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

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

[Rosen Penev]

On Fri, May 29, 2026 at 8:13 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [Critical] Unchecked array access in `mpc512x_psc_alloc_clock` and related clock functions where `psc_num` derived from device tree is used to index arrays, causing out-of-bounds reads and writes.
> --
>
> commit 7a3c433b5cd814a1fd0d2b57113cd41cb03279a5
> Author: Rosen Penev <rosenp@gmail.com>
>
> bestcomm/gen_bd: fix out-of-bounds access in PSC parameter lookup
>
> This commit fixes an out-of-bounds access issue in bcom_psc_gen_bd_rx_init()
> and bcom_psc_gen_bd_tx_init() by properly using ARRAY_SIZE() for bounds
> checking on psc_num. It also removes an unused forward declaration in
> bcom_psc_gen_bd_tx_init().
>
> > diff --git a/drivers/dma/bestcomm/gen_bd.c b/drivers/dma/bestcomm/gen_bd.c
> > index 8a24a5cbc2633..75d1a6f543df9 100644
> > --- a/drivers/dma/bestcomm/gen_bd.c
> > +++ b/drivers/dma/bestcomm/gen_bd.c
> > @@ -315,7 +315,7 @@ static struct 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))
>
> [Severity: Critical]
> This is a pre-existing issue, but while looking at this fix for the psc_num
> bounds checking, I noticed a similar unchecked array access regression vector
> using psc_num in drivers/tty/serial/mpc52xx_uart.c.
cross driver review. Impressive. How is anyone supposed to get anything merged?
>
> In mpc512x_psc_alloc_clock(), psc_num is calculated directly from
> port->mapbase:
>
> drivers/tty/serial/mpc52xx_uart.c:mpc512x_psc_alloc_clock() {
>         ...
>         psc_num = (port->mapbase & 0xf00) >> 8;
>         ...
>         psc_mclk_clk[psc_num] = clk;
>         ...
> }
>
> Because port->mapbase comes from the device tree via
> of_address_to_resource(), a malformed device tree could supply an address
> like 0x11F00, allowing psc_num to evaluate up to 15.
>
> Since psc_mclk_clk and psc_ipg_clk are arrays sized to MPC52xx_PSC_MAXNUM
> (which is 12 when CONFIG_PPC_MPC512x is defined), wouldn't this allow an
> out-of-bounds write of a pointer to the BSS section?
>
> The exact same unchecked psc_num calculation also appears in
> mpc512x_psc_relse_clock() and mpc512x_psc_endis_clock(), leading to similar
> out-of-bounds reads and writes.
>
> >               return NULL;
> >
> >       return bcom_gen_bd_rx_init(queue_len, fifo,
> [ ... ]
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260530030152.49759-1-rosenp@gmail.com?part=1