linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] dmaengine: ti: edma: Fix memory allocation size for queue_priority_map
@ 2025-08-29 13:13 Anders Roxell
  2025-08-29 23:21 ` Nathan Chancellor
  0 siblings, 1 reply; 5+ messages in thread
From: Anders Roxell @ 2025-08-29 13:13 UTC (permalink / raw)
  To: peter.ujfalusi, vkoul, nathan
  Cc: dmaengine, linux-kernel, llvm, dan.carpenter, arnd,
	benjamin.copeland, Anders Roxell

Fix a critical memory allocation bug in edma_setup_from_hw() where
queue_priority_map was allocated with insufficient memory. The code
declared queue_priority_map as s8 (*)[2] (pointer to array of 2 s8), but
allocated memory using sizeof(s8) instead of sizeof(s8[2]).

This caused out-of-bounds memory writes when accessing:
  queue_priority_map[i][0] = i;
  queue_priority_map[i][1] = i;

The bug manifested as kernel crashes with "Oops - undefined instruction"
on ARM platforms (BeagleBoard-X15) during EDMA driver probe, as the
memory corruption triggered kernel hardening features on Clang.

Change the allocation from:
  devm_kcalloc(dev, ecc->num_tc + 1, sizeof(s8), GFP_KERNEL)
to this:
  devm_kcalloc(dev, ecc->num_tc + 1, sizeof(s8[2]), GFP_KERNEL)

This ensures proper allocation of (ecc->num_tc + 1) * 2 bytes to match
the expected 2D array structure.

Fixes: 2b6b3b742019 ("ARM/dmaengine: edma: Merge the two drivers under drivers/dma/")
Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
---
 drivers/dma/ti/edma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/ti/edma.c b/drivers/dma/ti/edma.c
index 3ed406f08c44..8f9b65e4bc87 100644
--- a/drivers/dma/ti/edma.c
+++ b/drivers/dma/ti/edma.c
@@ -2064,7 +2064,7 @@ static int edma_setup_from_hw(struct device *dev, struct edma_soc_info *pdata,
 	 * priority. So Q0 is the highest priority queue and the last queue has
 	 * the lowest priority.
 	 */
-	queue_priority_map = devm_kcalloc(dev, ecc->num_tc + 1, sizeof(s8),
+	queue_priority_map = devm_kcalloc(dev, ecc->num_tc + 1, sizeof(s8[2]),
 					  GFP_KERNEL);
 	if (!queue_priority_map)
 		return -ENOMEM;
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] dmaengine: ti: edma: Fix memory allocation size for queue_priority_map
  2025-08-29 13:13 [PATCH] dmaengine: ti: edma: Fix memory allocation size for queue_priority_map Anders Roxell
@ 2025-08-29 23:21 ` Nathan Chancellor
  2025-08-30  9:48   ` Anders Roxell
  2025-08-30  9:49   ` [PATCHv2] " Anders Roxell
  0 siblings, 2 replies; 5+ messages in thread
From: Nathan Chancellor @ 2025-08-29 23:21 UTC (permalink / raw)
  To: Anders Roxell
  Cc: peter.ujfalusi, vkoul, dmaengine, linux-kernel, llvm,
	dan.carpenter, arnd, benjamin.copeland

Hi Anders,

On Fri, Aug 29, 2025 at 03:13:46PM +0200, Anders Roxell wrote:
> Fix a critical memory allocation bug in edma_setup_from_hw() where
> queue_priority_map was allocated with insufficient memory. The code
> declared queue_priority_map as s8 (*)[2] (pointer to array of 2 s8), but
> allocated memory using sizeof(s8) instead of sizeof(s8[2]).
> 
> This caused out-of-bounds memory writes when accessing:
>   queue_priority_map[i][0] = i;
>   queue_priority_map[i][1] = i;
> 
> The bug manifested as kernel crashes with "Oops - undefined instruction"
> on ARM platforms (BeagleBoard-X15) during EDMA driver probe, as the
> memory corruption triggered kernel hardening features on Clang.
> 
> Change the allocation from:
>   devm_kcalloc(dev, ecc->num_tc + 1, sizeof(s8), GFP_KERNEL)
> to this:
>   devm_kcalloc(dev, ecc->num_tc + 1, sizeof(s8[2]), GFP_KERNEL)
> 
> This ensures proper allocation of (ecc->num_tc + 1) * 2 bytes to match
> the expected 2D array structure.
> 
> Fixes: 2b6b3b742019 ("ARM/dmaengine: edma: Merge the two drivers under drivers/dma/")
> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
> ---
>  drivers/dma/ti/edma.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/dma/ti/edma.c b/drivers/dma/ti/edma.c
> index 3ed406f08c44..8f9b65e4bc87 100644
> --- a/drivers/dma/ti/edma.c
> +++ b/drivers/dma/ti/edma.c
> @@ -2064,7 +2064,7 @@ static int edma_setup_from_hw(struct device *dev, struct edma_soc_info *pdata,
>  	 * priority. So Q0 is the highest priority queue and the last queue has
>  	 * the lowest priority.
>  	 */
> -	queue_priority_map = devm_kcalloc(dev, ecc->num_tc + 1, sizeof(s8),
> +	queue_priority_map = devm_kcalloc(dev, ecc->num_tc + 1, sizeof(s8[2]),

Would

  sizeof(*queue_priority_map)

work instead? That tends to be preferred within the kernel so that the
type information is not open coded twice and it helps avoid bugs exactly
like this one. See other uses of devm_kcalloc() and "14) Allocating
memory" in Documentation/process/coding-style.rst.

>  					  GFP_KERNEL);
>  	if (!queue_priority_map)
>  		return -ENOMEM;
> -- 
> 2.50.1
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] dmaengine: ti: edma: Fix memory allocation size for queue_priority_map
  2025-08-29 23:21 ` Nathan Chancellor
@ 2025-08-30  9:48   ` Anders Roxell
  2025-08-30  9:49   ` [PATCHv2] " Anders Roxell
  1 sibling, 0 replies; 5+ messages in thread
From: Anders Roxell @ 2025-08-30  9:48 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: peter.ujfalusi, vkoul, dmaengine, linux-kernel, llvm,
	dan.carpenter, arnd, benjamin.copeland

On Sat, 30 Aug 2025 at 01:21, Nathan Chancellor <nathan@kernel.org> wrote:
>
> Hi Anders,
>
> On Fri, Aug 29, 2025 at 03:13:46PM +0200, Anders Roxell wrote:
> > Fix a critical memory allocation bug in edma_setup_from_hw() where
> > queue_priority_map was allocated with insufficient memory. The code
> > declared queue_priority_map as s8 (*)[2] (pointer to array of 2 s8), but
> > allocated memory using sizeof(s8) instead of sizeof(s8[2]).
> >
> > This caused out-of-bounds memory writes when accessing:
> >   queue_priority_map[i][0] = i;
> >   queue_priority_map[i][1] = i;
> >
> > The bug manifested as kernel crashes with "Oops - undefined instruction"
> > on ARM platforms (BeagleBoard-X15) during EDMA driver probe, as the
> > memory corruption triggered kernel hardening features on Clang.
> >
> > Change the allocation from:
> >   devm_kcalloc(dev, ecc->num_tc + 1, sizeof(s8), GFP_KERNEL)
> > to this:
> >   devm_kcalloc(dev, ecc->num_tc + 1, sizeof(s8[2]), GFP_KERNEL)
> >
> > This ensures proper allocation of (ecc->num_tc + 1) * 2 bytes to match
> > the expected 2D array structure.
> >
> > Fixes: 2b6b3b742019 ("ARM/dmaengine: edma: Merge the two drivers under drivers/dma/")
> > Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
> > ---
> >  drivers/dma/ti/edma.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/dma/ti/edma.c b/drivers/dma/ti/edma.c
> > index 3ed406f08c44..8f9b65e4bc87 100644
> > --- a/drivers/dma/ti/edma.c
> > +++ b/drivers/dma/ti/edma.c
> > @@ -2064,7 +2064,7 @@ static int edma_setup_from_hw(struct device *dev, struct edma_soc_info *pdata,
> >        * priority. So Q0 is the highest priority queue and the last queue has
> >        * the lowest priority.
> >        */
> > -     queue_priority_map = devm_kcalloc(dev, ecc->num_tc + 1, sizeof(s8),
> > +     queue_priority_map = devm_kcalloc(dev, ecc->num_tc + 1, sizeof(s8[2]),
>
> Would
>
>   sizeof(*queue_priority_map)
>
> work instead? That tends to be preferred within the kernel so that the
> type information is not open coded twice and it helps avoid bugs exactly
> like this one. See other uses of devm_kcalloc() and "14) Allocating
> memory" in Documentation/process/coding-style.rst.

Thank you Nathan for the review, that makes sense. I’ll send a v2 shortly.

Cheers,
Anders

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCHv2] dmaengine: ti: edma: Fix memory allocation size for queue_priority_map
  2025-08-29 23:21 ` Nathan Chancellor
  2025-08-30  9:48   ` Anders Roxell
@ 2025-08-30  9:49   ` Anders Roxell
  2025-09-02  9:35     ` Vinod Koul
  1 sibling, 1 reply; 5+ messages in thread
From: Anders Roxell @ 2025-08-30  9:49 UTC (permalink / raw)
  To: peter.ujfalusi, vkoul, nathan
  Cc: dmaengine, linux-kernel, llvm, dan.carpenter, arnd,
	benjamin.copeland, Anders Roxell

Fix a critical memory allocation bug in edma_setup_from_hw() where
queue_priority_map was allocated with insufficient memory. The code
declared queue_priority_map as s8 (*)[2] (pointer to array of 2 s8),
but allocated memory using sizeof(s8) instead of the correct size.

This caused out-of-bounds memory writes when accessing:
  queue_priority_map[i][0] = i;
  queue_priority_map[i][1] = i;

The bug manifested as kernel crashes with "Oops - undefined instruction"
on ARM platforms (BeagleBoard-X15) during EDMA driver probe, as the
memory corruption triggered kernel hardening features on Clang.

Change the allocation to use sizeof(*queue_priority_map) which
automatically gets the correct size for the 2D array structure.

Fixes: 2b6b3b742019 ("ARM/dmaengine: edma: Merge the two drivers under drivers/dma/")
Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
---
 drivers/dma/ti/edma.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/ti/edma.c b/drivers/dma/ti/edma.c
index 3ed406f08c44..552be71db6c4 100644
--- a/drivers/dma/ti/edma.c
+++ b/drivers/dma/ti/edma.c
@@ -2064,8 +2064,8 @@ static int edma_setup_from_hw(struct device *dev, struct edma_soc_info *pdata,
 	 * priority. So Q0 is the highest priority queue and the last queue has
 	 * the lowest priority.
 	 */
-	queue_priority_map = devm_kcalloc(dev, ecc->num_tc + 1, sizeof(s8),
-					  GFP_KERNEL);
+	queue_priority_map = devm_kcalloc(dev, ecc->num_tc + 1,
+					  sizeof(*queue_priority_map), GFP_KERNEL);
 	if (!queue_priority_map)
 		return -ENOMEM;
 
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCHv2] dmaengine: ti: edma: Fix memory allocation size for queue_priority_map
  2025-08-30  9:49   ` [PATCHv2] " Anders Roxell
@ 2025-09-02  9:35     ` Vinod Koul
  0 siblings, 0 replies; 5+ messages in thread
From: Vinod Koul @ 2025-09-02  9:35 UTC (permalink / raw)
  To: peter.ujfalusi, nathan, Anders Roxell
  Cc: dmaengine, linux-kernel, llvm, dan.carpenter, arnd,
	benjamin.copeland


On Sat, 30 Aug 2025 11:49:53 +0200, Anders Roxell wrote:
> Fix a critical memory allocation bug in edma_setup_from_hw() where
> queue_priority_map was allocated with insufficient memory. The code
> declared queue_priority_map as s8 (*)[2] (pointer to array of 2 s8),
> but allocated memory using sizeof(s8) instead of the correct size.
> 
> This caused out-of-bounds memory writes when accessing:
>   queue_priority_map[i][0] = i;
>   queue_priority_map[i][1] = i;
> 
> [...]

Applied, thanks!

[1/1] dmaengine: ti: edma: Fix memory allocation size for queue_priority_map
      commit: e63419dbf2ceb083c1651852209c7f048089ac0f

Best regards,
-- 
~Vinod



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-09-02  9:35 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-29 13:13 [PATCH] dmaengine: ti: edma: Fix memory allocation size for queue_priority_map Anders Roxell
2025-08-29 23:21 ` Nathan Chancellor
2025-08-30  9:48   ` Anders Roxell
2025-08-30  9:49   ` [PATCHv2] " Anders Roxell
2025-09-02  9:35     ` Vinod Koul

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).