[PATCHv2] dma: bestcomm: gen_bd: split struct bcom_psc_params from array definition

[PATCHv2] dma: 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>
---
 v2: add const and add variable descriptions
 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 8a24a5cbc263..61b5746e1a97 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.54.0

Re: [PATCHv2] dma: bestcomm: gen_bd: split struct bcom_psc_params from array definition

[sashiko-bot]

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

Pre-existing issues:
- [High] Inadequate or missing bounds checking on `psc_num` can lead to out-of-bounds array access in `bcom_psc_params`.
--

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

dma: bestcomm: gen_bd: split struct bcom_psc_params from array definition
    
This commit splits the bcom_psc_params struct definition from its array
declaration to resolve kernel-doc parser warnings. It also adds missing
members and marks the lookup table 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
> @@ -254,17 +254,23 @@ EXPORT_SYMBOL_GPL(bcom_gen_bd_tx_release);

[ ... ]

> -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 this fixed-size array expose an
out-of-bounds access?

The array strictly contains 6 elements (indices 0 through 5). However,
bcom_psc_gen_bd_rx_init() checks the psc_num index against
MPC52xx_PSC_MAXNUM:

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

        ...
        bcom_psc_params[psc_num].rx_initiator,
        ...
}

If CONFIG_PPC_MPC512x is enabled, MPC52xx_PSC_MAXNUM can be 12, allowing an
index up to 11 to access the 6-element bcom_psc_params[].

Additionally, bcom_psc_gen_bd_tx_init() appears to perform no bounds checking
on psc_num at all before indexing this array. Callers like 
mpc5200_audio_dma_create() can pass an unvalidated cell-index from the
device tree directly into these functions.

Can a malformed device tree trigger out-of-bounds memory reads here, leading
to out-of-bounds MMIO writes when using the read garbage as the initiator
and ipr?

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

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

Re: [PATCHv2] dma: bestcomm: gen_bd: split struct bcom_psc_params from array definition

[Rosen Penev]

On Fri, May 29, 2026 at 7:19 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Inadequate or missing bounds checking on `psc_num` can lead to out-of-bounds array access in `bcom_psc_params`.
> --
>
> commit c24864efcf21b2526be68d727e70076f6cee258c
> Author: Rosen Penev <rosenp@gmail.com>
>
> dma: bestcomm: gen_bd: split struct bcom_psc_params from array definition
>
> This commit splits the bcom_psc_params struct definition from its array
> declaration to resolve kernel-doc parser warnings. It also adds missing
> members and marks the lookup table 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
> > @@ -254,17 +254,23 @@ EXPORT_SYMBOL_GPL(bcom_gen_bd_tx_release);
>
> [ ... ]
>
> > -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 this fixed-size array expose an
> out-of-bounds access?
>
> The array strictly contains 6 elements (indices 0 through 5). However,
> bcom_psc_gen_bd_rx_init() checks the psc_num index against
> MPC52xx_PSC_MAXNUM:
>
> drivers/dma/bestcomm/gen_bd.c:bcom_psc_gen_bd_rx_init() {
>         if (psc_num >= MPC52xx_PSC_MAXNUM)
>                 return NULL;
>
>         ...
>         bcom_psc_params[psc_num].rx_initiator,
>         ...
> }
>
> If CONFIG_PPC_MPC512x is enabled, MPC52xx_PSC_MAXNUM can be 12, allowing an
> index up to 11 to access the 6-element bcom_psc_params[].
>
> Additionally, bcom_psc_gen_bd_tx_init() appears to perform no bounds checking
> on psc_num at all before indexing this array. Callers like
> mpc5200_audio_dma_create() can pass an unvalidated cell-index from the
> device tree directly into these functions.
>
> Can a malformed device tree trigger out-of-bounds memory reads here, leading
> to out-of-bounds MMIO writes when using the read garbage as the initiator
> and ipr?
Fixed in different patch.
>
> >       [0] = {
> >               .rx_initiator = BCOM_INITIATOR_PSC1_RX,
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260530012554.68605-1-rosenp@gmail.com?part=1

[PATCHv2] dma: 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>
---
 v2: add const and add variable descriptions
 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 8a24a5cbc263..61b5746e1a97 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.54.0