bestcomm/gen_bd: fix out-of-bounds access in PSC parameter lookup
PATCH 1/1 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.
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.
--- drivers/dma/bestcomm/gen_bd.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/dma/bestcomm/gen_bd.c +4 -2
@@ -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);
Signature · 1 line
2.54.0
[email protected]
AI reviewThank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
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 <[email protected]>
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().
drivers/dma/bestcomm/gen_bd.c static struct bcom_psc_params { · line 318
Quoted 10 lines
drivers/dma/bestcomm/gen_bd.c +1
-1@@ -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.
Quoted 3 lines
drivers/dma/bestcomm/gen_bd.c +0
-0return NULL ; return bcom_gen_bd_rx_init(queue_len, fifo,
[ ... ]
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
Quoted 17 lines · On Fri, May 29, 2026 at 8:13 PM <[email protected]> wrote:
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Unchecked array access inmpc512x_psc_alloc_clockand related clock functions wherepsc_numderived from device tree is used to index arrays, causing out-of-bounds reads and writes. --commit
7a3c433b5cd814a1fd0d2b57113cd41cb03279a5
Author: Rosen Penev <[email protected]>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()andbcom_psc_gen_bd_tx_init()by properly usingARRAY_SIZE()for bounds checking onpsc_num. It also removes an unused forward declaration inbcom_psc_gen_bd_tx_init().
Quoted 15 lines
drivers/dma/bestcomm/gen_bd.c static struct bcom_psc_params { · line 318
Quoted 10 lines
drivers/dma/bestcomm/gen_bd.c +1
-1@@ -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 thepsc_numbounds checking, I noticed a similar unchecked array access regression vector usingpsc_numindrivers/tty/serial/mpc52xx_uart.c.
cross driver review. Impressive. How is anyone supposed to get anything merged?
Quoted 31 lines
In
mpc512x_psc_alloc_clock(),psc_numis calculated directly fromport->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->mapbasecomes from the device tree viaof_address_to_resource(), a malformed device tree could supply an address like0x11F00, allowingpsc_numto evaluate up to 15.Since
psc_mclk_clkandpsc_ipg_clkare arrays sized toMPC52xx_PSC_MAXNUM(which is 12 whenCONFIG_PPC_MPC512xis defined), wouldn't this allow an out-of-bounds write of a pointer to the BSS section?The exact same unchecked
psc_numcalculation also appears inmpc512x_psc_relse_clock()andmpc512x_psc_endis_clock(), leading to similar out-of-bounds reads and writes.drivers/dma/bestcomm/gen_bd.c static const struct bcom_psc_params bcom_psc_params[] = { · line 327
Quoted 3 lines
drivers/dma/bestcomm/gen_bd.c +0
-0return NULL ; return bcom_gen_bd_rx_init(queue_len, fifo,[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1