[PATCH] dmaengine: bestcomm: gen_bd: split struct bcom_psc_params from array definition

[PATCH] dmaengine: bestcomm: gen_bd: split struct bcom_psc_params from array definition

[Rosen Penev]

The combined struct-definition-with-initializer pattern confuses the
kernel-doc parser. Split into separate struct definition and array
declaration.

Also now that it's fixed, it warns on missing members. Add those as
well.

Since this is just a lookup table and not modified, make it const so
that it can be moved to read only memory.

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

diff --git a/drivers/dma/bestcomm/gen_bd.c b/drivers/dma/bestcomm/gen_bd.c
index 75d1a6f543df..00dfdc4b8eba 100644
--- a/drivers/dma/bestcomm/gen_bd.c
+++ b/drivers/dma/bestcomm/gen_bd.c
@@ -254,17 +254,23 @@ EXPORT_SYMBOL_GPL(bcom_gen_bd_tx_release);
  */
 
 /**
- * bcom_psc_parameters - Bestcomm initialization value table for PSC devices
+ * struct bcom_psc_params - Bestcomm initialization value table for PSC devices
+ * @rx_initiator: RX initiator ID
+ * @rx_ipr: RX interrupt priority register value
+ * @tx_initiator: TX initiator ID
+ * @tx_ipr: TX interrupt priority register value
  *
  * This structure is only used internally.  It is a lookup table for PSC
  * specific parameters to bestcomm tasks.
  */
-static struct bcom_psc_params {
+struct bcom_psc_params {
 	int rx_initiator;
 	int rx_ipr;
 	int tx_initiator;
 	int tx_ipr;
-} bcom_psc_params[] = {
+};
+
+static const struct bcom_psc_params bcom_psc_params[] = {
 	[0] = {
 		.rx_initiator = BCOM_INITIATOR_PSC1_RX,
 		.rx_ipr = BCOM_IPR_PSC1_RX,
-- 
2.55.0

Re: [PATCH] dmaengine: bestcomm: gen_bd: split struct bcom_psc_params from array definition

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The function `bcom_psc_gen_bd_tx_init` lacks bounds checking for `psc_num` before using it as an index into the `bcom_psc_params` array.
- [High] The bounds check in `bcom_psc_gen_bd_rx_init` uses a macro (`MPC52xx_PSC_MAXNUM`) that can exceed the actual size of the `bcom_psc_params` array.
--

--- Patch [1]: [PATCH] dmaengine: bestcomm: gen_bd: split struct bcom_psc_params from array definition ---
commit 2de5fbd01540d512aabb05b4b1f3cc916af97378
Author: Rosen Penev <rosenp@gmail.com>

dmaengine: bestcomm: gen_bd: split struct bcom_psc_params from array definition

This commit separates the bcom_psc_params struct definition from its array
initialization to fix kernel-doc parsing issues. It also adds documentation
for missing members and marks the array as const so it can be moved to
read-only memory.

> diff --git a/drivers/dma/bestcomm/gen_bd.c b/drivers/dma/bestcomm/gen_bd.c
> index 8a24a5cbc2633..61b5746e1a972 100644
> --- a/drivers/dma/bestcomm/gen_bd.c
> +++ b/drivers/dma/bestcomm/gen_bd.c

[ ... ]

> -static struct bcom_psc_params {
> +struct bcom_psc_params {
>  	int rx_initiator;
>  	int rx_ipr;
>  	int tx_initiator;
>  	int tx_ipr;
> -} bcom_psc_params[] = {
> +};
> +
> +static const struct bcom_psc_params bcom_psc_params[] = {

[Severity: High]
This is a pre-existing issue, but does the bounds check in
bcom_psc_gen_bd_rx_init() adequately protect this 6-element array?

drivers/dma/bestcomm/gen_bd.c:bcom_psc_gen_bd_rx_init() {
    if (psc_num >= MPC52xx_PSC_MAXNUM)
        return NULL;

    return bcom_gen_bd_rx_init(queue_len, fifo,
                               bcom_psc_params[psc_num].rx_initiator,
                               bcom_psc_params[psc_num].rx_ipr,
                               maxbufsize);
}

If a multi-platform kernel has CONFIG_PPC_MPC512x enabled,
MPC52xx_PSC_MAXNUM evaluates to 12.

Since this array explicitly only contains 6 elements (indices 0 to 5), a
cell-index between 6 and 11 from a malformed device tree parsed in
mpc5200_audio_dma_create() would bypass the check and read out-of-bounds.

This could cause bcom_gen_bd_rx_init() to write out-of-bounds to the IPR
register array.

[Severity: High]
This is also a pre-existing issue, but is there a missing bounds check when
using psc_num to index this array in bcom_psc_gen_bd_tx_init()?

drivers/dma/bestcomm/gen_bd.c:bcom_psc_gen_bd_tx_init() {
    ...
    return bcom_gen_bd_tx_init(queue_len, fifo,
                               bcom_psc_params[psc_num].tx_initiator,
                               bcom_psc_params[psc_num].tx_ipr);
}

An unvalidated psc_num from a device tree cell-index parsed in
mpc5200_audio_dma_create() could cause an out-of-bounds read here.

The resulting garbage data passed to bcom_gen_bd_tx_init() could then lead
to an out-of-bounds write to the IPR register array, potentially causing
memory corruption.

>  	[0] = {
>  		.rx_initiator = BCOM_INITIATOR_PSC1_RX,
>  		.rx_ipr = BCOM_IPR_PSC1_RX,

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