* Re: [PATCHv2] dmaengine: bestcomm: make gen_bd init helpers static
2026-09-13 19:48 [PATCHv2] dmaengine: bestcomm: make gen_bd init helpers static Rosen Penev
@ 2026-09-13 19:57 ` sashiko-bot
2026-09-14 5:51 ` Christophe Leroy (CS GROUP)
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-09-13 19:57 UTC (permalink / raw)
To: Rosen Penev; +Cc: vkoul, Frank.Li, dmaengine
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing bounds check for `psc_num` in `bcom_psc_gen_bd_tx_init` can lead to an out-of-bounds read on the `bcom_psc_params` array, resulting in a subsequent out-of-bounds memory write.
--
commit 208dbbe4b8a05c7464d2b55c9abc2d1a2ba7e83a
Author: Rosen Penev <rosenp@gmail.com>
dmaengine: bestcomm: make gen_bd init helpers static
bcom_gen_bd_rx_init() and bcom_gen_bd_tx_init() are only called from
the PSC wrapper functions in gen_bd.c, so make them static, drop
their now-unneeded exports, and remove the corresponding header
declarations.
> diff --git a/drivers/dma/bestcomm/gen_bd.c b/drivers/dma/bestcomm/gen_bd.c
> index 8a24a5cbc2633..abe6c19f7148e 100644
> --- a/drivers/dma/bestcomm/gen_bd.c
> +++ b/drivers/dma/bestcomm/gen_bd.c
[ ... ]
> @@ -166,7 +165,7 @@ bcom_gen_bd_rx_release(struct bcom_task *tsk)
> EXPORT_SYMBOL_GPL(bcom_gen_bd_rx_release);
>
>
> -extern struct bcom_task *
> +static struct bcom_task *
> bcom_gen_bd_tx_init(int queue_len, phys_addr_t fifo,
> int initiator, int ipr)
> {
> @@ -192,7 +191,6 @@ bcom_gen_bd_tx_init(int queue_len, phys_addr_t fifo,
>
> return tsk;
> }
> -EXPORT_SYMBOL_GPL(bcom_gen_bd_tx_init);
[Severity: High]
This is a pre-existing issue, but does bcom_psc_gen_bd_tx_init() lack a bounds
check for psc_num before calling this function?
While bcom_psc_gen_bd_rx_init() checks if psc_num is out of bounds:
if (psc_num >= MPC52xx_PSC_MAXNUM)
return NULL;
bcom_psc_gen_bd_tx_init() does not, leading to a direct out-of-bounds read on
the statically sized bcom_psc_params array:
return bcom_gen_bd_tx_init(queue_len, fifo,
bcom_psc_params[psc_num].tx_initiator,
bcom_psc_params[psc_num].tx_ipr);
The garbage tx_initiator value read out-of-bounds is then passed to
bcom_gen_bd_tx_init() and subsequently used in bcom_gen_bd_tx_reset() as an
array index for an MMIO write:
out_8(&bcom_eng->regs->ipr[priv->initiator], priv->ipr);
This can be triggered via mpc5200_audio_dma_create() in
sound/soc/fsl/mpc5200_dma.c, where the caller blindly passes the id derived
from the device-tree 'cell-index' property:
psc_dma->playback.bcom_task =
bcom_psc_gen_bd_tx_init(psc_dma->id, 10, fifo);
Since it calls tx_init before checking the result of rx_init, the out-of-bounds
read and write will unconditionally execute on a bad ID before any error path
can bail out.
Could a bounds check be added to bcom_psc_gen_bd_tx_init()?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260913194804.39939-1-rosenp@gmail.com?part=1
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCHv2] dmaengine: bestcomm: make gen_bd init helpers static
2026-09-13 19:48 [PATCHv2] dmaengine: bestcomm: make gen_bd init helpers static Rosen Penev
2026-09-13 19:57 ` sashiko-bot
@ 2026-09-14 5:51 ` Christophe Leroy (CS GROUP)
2026-09-14 14:23 ` Frank Li
2026-09-15 17:36 ` Vinod Koul
3 siblings, 0 replies; 5+ messages in thread
From: Christophe Leroy (CS GROUP) @ 2026-09-14 5:51 UTC (permalink / raw)
To: Rosen Penev, dmaengine
Cc: Vinod Koul, Frank Li, open list, open list:FREESCALE SOC DRIVERS,
moderated list:FREESCALE SOC DRIVERS
Le 13/09/2026 à 21:48, Rosen Penev a écrit :
> bcom_gen_bd_rx_init() and bcom_gen_bd_tx_init() are only called from
> the PSC wrapper functions in gen_bd.c, so make them static, drop
> their now-unneeded exports, and remove the corresponding header
> declarations.
>
> These had use before commit 7d3ee229ea51
> ("powerpc: Drop MPC5200 LocalPlus bus FIFO driver")
>
> Assisted-by: LLM
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
Reviewed-by: Christophe Leroy (CS GROUP) <chleroy@kernel.org>
> ---
> v2: mention driver removal that made them unused.
> drivers/dma/bestcomm/gen_bd.c | 6 ++----
> include/linux/fsl/bestcomm/gen_bd.h | 8 --------
> 2 files changed, 2 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/dma/bestcomm/gen_bd.c b/drivers/dma/bestcomm/gen_bd.c
> index 61b5746e1a97..b5093f7445cb 100644
> --- a/drivers/dma/bestcomm/gen_bd.c
> +++ b/drivers/dma/bestcomm/gen_bd.c
> @@ -81,7 +81,7 @@ struct bcom_gen_bd_priv {
> /* Task support code */
> /* ======================================================================== */
>
> -struct bcom_task *
> +static struct bcom_task *
> bcom_gen_bd_rx_init(int queue_len, phys_addr_t fifo,
> int initiator, int ipr, int maxbufsize)
> {
> @@ -108,7 +108,6 @@ bcom_gen_bd_rx_init(int queue_len, phys_addr_t fifo,
>
> return tsk;
> }
> -EXPORT_SYMBOL_GPL(bcom_gen_bd_rx_init);
>
> int
> bcom_gen_bd_rx_reset(struct bcom_task *tsk)
> @@ -166,7 +165,7 @@ bcom_gen_bd_rx_release(struct bcom_task *tsk)
> EXPORT_SYMBOL_GPL(bcom_gen_bd_rx_release);
>
>
> -extern struct bcom_task *
> +static struct bcom_task *
> bcom_gen_bd_tx_init(int queue_len, phys_addr_t fifo,
> int initiator, int ipr)
> {
> @@ -192,7 +191,6 @@ bcom_gen_bd_tx_init(int queue_len, phys_addr_t fifo,
>
> return tsk;
> }
> -EXPORT_SYMBOL_GPL(bcom_gen_bd_tx_init);
>
> int
> bcom_gen_bd_tx_reset(struct bcom_task *tsk)
> diff --git a/include/linux/fsl/bestcomm/gen_bd.h b/include/linux/fsl/bestcomm/gen_bd.h
> index aeb312a1cd00..7253d308d77a 100644
> --- a/include/linux/fsl/bestcomm/gen_bd.h
> +++ b/include/linux/fsl/bestcomm/gen_bd.h
> @@ -16,10 +16,6 @@ struct bcom_gen_bd {
> };
>
>
> -extern struct bcom_task *
> -bcom_gen_bd_rx_init(int queue_len, phys_addr_t fifo,
> - int initiator, int ipr, int maxbufsize);
> -
> extern int
> bcom_gen_bd_rx_reset(struct bcom_task *tsk);
>
> @@ -27,10 +23,6 @@ extern void
> bcom_gen_bd_rx_release(struct bcom_task *tsk);
>
>
> -extern struct bcom_task *
> -bcom_gen_bd_tx_init(int queue_len, phys_addr_t fifo,
> - int initiator, int ipr);
> -
> extern int
> bcom_gen_bd_tx_reset(struct bcom_task *tsk);
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCHv2] dmaengine: bestcomm: make gen_bd init helpers static
2026-09-13 19:48 [PATCHv2] dmaengine: bestcomm: make gen_bd init helpers static Rosen Penev
2026-09-13 19:57 ` sashiko-bot
2026-09-14 5:51 ` Christophe Leroy (CS GROUP)
@ 2026-09-14 14:23 ` Frank Li
2026-09-15 17:36 ` Vinod Koul
3 siblings, 0 replies; 5+ messages in thread
From: Frank Li @ 2026-09-14 14:23 UTC (permalink / raw)
To: Rosen Penev
Cc: dmaengine, Vinod Koul, Frank Li, Christophe Leroy (CS GROUP),
open list, open list:FREESCALE SOC DRIVERS,
moderated list:FREESCALE SOC DRIVERS
On Sun, Sep 13, 2026 at 12:48:04PM -0700, Rosen Penev wrote:
> bcom_gen_bd_rx_init() and bcom_gen_bd_tx_init() are only called from
> the PSC wrapper functions in gen_bd.c, so make them static, drop
> their now-unneeded exports, and remove the corresponding header
> declarations.
>
> These had use before commit 7d3ee229ea51
> ("powerpc: Drop MPC5200 LocalPlus bus FIFO driver")
>
> Assisted-by: LLM
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
Reviewed-by: Frank Li <Frank.Li@nxp.com>
> v2: mention driver removal that made them unused.
> drivers/dma/bestcomm/gen_bd.c | 6 ++----
> include/linux/fsl/bestcomm/gen_bd.h | 8 --------
> 2 files changed, 2 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/dma/bestcomm/gen_bd.c b/drivers/dma/bestcomm/gen_bd.c
> index 61b5746e1a97..b5093f7445cb 100644
> --- a/drivers/dma/bestcomm/gen_bd.c
> +++ b/drivers/dma/bestcomm/gen_bd.c
> @@ -81,7 +81,7 @@ struct bcom_gen_bd_priv {
> /* Task support code */
> /* ======================================================================== */
>
> -struct bcom_task *
> +static struct bcom_task *
> bcom_gen_bd_rx_init(int queue_len, phys_addr_t fifo,
> int initiator, int ipr, int maxbufsize)
> {
> @@ -108,7 +108,6 @@ bcom_gen_bd_rx_init(int queue_len, phys_addr_t fifo,
>
> return tsk;
> }
> -EXPORT_SYMBOL_GPL(bcom_gen_bd_rx_init);
>
> int
> bcom_gen_bd_rx_reset(struct bcom_task *tsk)
> @@ -166,7 +165,7 @@ bcom_gen_bd_rx_release(struct bcom_task *tsk)
> EXPORT_SYMBOL_GPL(bcom_gen_bd_rx_release);
>
>
> -extern struct bcom_task *
> +static struct bcom_task *
> bcom_gen_bd_tx_init(int queue_len, phys_addr_t fifo,
> int initiator, int ipr)
> {
> @@ -192,7 +191,6 @@ bcom_gen_bd_tx_init(int queue_len, phys_addr_t fifo,
>
> return tsk;
> }
> -EXPORT_SYMBOL_GPL(bcom_gen_bd_tx_init);
>
> int
> bcom_gen_bd_tx_reset(struct bcom_task *tsk)
> diff --git a/include/linux/fsl/bestcomm/gen_bd.h b/include/linux/fsl/bestcomm/gen_bd.h
> index aeb312a1cd00..7253d308d77a 100644
> --- a/include/linux/fsl/bestcomm/gen_bd.h
> +++ b/include/linux/fsl/bestcomm/gen_bd.h
> @@ -16,10 +16,6 @@ struct bcom_gen_bd {
> };
>
>
> -extern struct bcom_task *
> -bcom_gen_bd_rx_init(int queue_len, phys_addr_t fifo,
> - int initiator, int ipr, int maxbufsize);
> -
> extern int
> bcom_gen_bd_rx_reset(struct bcom_task *tsk);
>
> @@ -27,10 +23,6 @@ extern void
> bcom_gen_bd_rx_release(struct bcom_task *tsk);
>
>
> -extern struct bcom_task *
> -bcom_gen_bd_tx_init(int queue_len, phys_addr_t fifo,
> - int initiator, int ipr);
> -
> extern int
> bcom_gen_bd_tx_reset(struct bcom_task *tsk);
>
> --
> 2.55.0
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCHv2] dmaengine: bestcomm: make gen_bd init helpers static
2026-09-13 19:48 [PATCHv2] dmaengine: bestcomm: make gen_bd init helpers static Rosen Penev
` (2 preceding siblings ...)
2026-09-14 14:23 ` Frank Li
@ 2026-09-15 17:36 ` Vinod Koul
3 siblings, 0 replies; 5+ messages in thread
From: Vinod Koul @ 2026-09-15 17:36 UTC (permalink / raw)
To: dmaengine, Rosen Penev
Cc: Frank Li, Christophe Leroy (CS GROUP), linux-kernel, linuxppc-dev,
linux-arm-kernel
On Sun, 13 Sep 2026 12:48:04 -0700, Rosen Penev wrote:
> bcom_gen_bd_rx_init() and bcom_gen_bd_tx_init() are only called from
> the PSC wrapper functions in gen_bd.c, so make them static, drop
> their now-unneeded exports, and remove the corresponding header
> declarations.
>
> These had use before commit 7d3ee229ea51
> ("powerpc: Drop MPC5200 LocalPlus bus FIFO driver")
>
> [...]
Applied, thanks!
[1/1] dmaengine: bestcomm: make gen_bd init helpers static
commit: 1ff9bed1b5547259f67e58b03a83eda934d78735
Best regards,
--
~Vinod
^ permalink raw reply [flat|nested] 5+ messages in thread