diff mbox series

[2/5] dmaengine: sun6i-dma: Add set_addr function pointer for variable address widths

Message ID 20260619-sun60i-a733-dma-v1-2-da4b649fc72a@gmail.com (mailing list archive)
State New
Headers
Series dmaengine: sun6i-dma: Add support for Allwinner A733 DMA controller |

Commit Message

Yuanshen Cao June 19, 2026, 4:53 a.m. UTC 
The A733 DMA controller supports higher address (up to 32G) compared to
previous generations. The existing `sun6i_dma_set_addr` function uses a
hardcoded logic for setting the high-address bits in the LLI parameters.

By moving `set_addr` into the `sun6i_dma_config` structure, we can
provide specialized implementations for different hardware. This allows
the A733 to use a version of `set_addr` that correctly handles its
specific `SRC_HIGH_ADDR_32G` and `DST_HIGH_ADDR_32G` in the `set_addr`
register later in the series.

Changes:
- Added `set_addr` function pointer to `struct sun6i_dma_config`.
- Refactored `sun6i_dma_set_addr` and introduced
  `sun6i_dma_set_addr_a100` (keeping the logic for high address
  support).
- Updated all existing configuration structs to include the new
  `set_addr` pointer.
- Removed `has_high_addr` since the logic is replaced by
  `sun6i_dma_set_addr_a100`.

Signed-off-by: Yuanshen Cao <alex.caoys@gmail.com>
---
 drivers/dma/sun6i-dma.c | 36 ++++++++++++++++++++++++++----------
 1 file changed, 26 insertions(+), 10 deletions(-)

Comments

sashiko-bot@kernel.org June 19, 2026, 5:03 a.m. UTC | #1 
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Use-after-free vulnerability in the error handling paths of `sun6i_dma_prep_slave_sg` and `sun6i_dma_prep_dma_cyclic` when freeing DMA pool items.
--

commit ea99a7147f9100709dbaea854d96eccdbc89f235
Author: Yuanshen Cao <alex.caoys@gmail.com>

dmaengine: sun6i-dma: Add set_addr function pointer for variable address widths

The A733 DMA controller supports higher address (up to 32G) compared to
previous generations. By moving set_addr into the sun6i_dma_config
structure, we can provide specialized implementations for different hardware.

> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index d92e702320d9f..059455425e19c 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
[ ... ]
> @@ -787,7 +796,7 @@ static struct dma_async_tx_descriptor *sun6i_dma_prep_slave_sg(
>  				sg_dma_len(sg), flags);
>  
>  		} else {
> -			sun6i_dma_set_addr(sdev, v_lli,
> +			sdev->cfg->set_addr(sdev, v_lli,
>  					   sconfig->src_addr,
>  					   sg_dma_address(sg));
>  			v_lli->cfg = lli_cfg;

[Severity: High]
This isn't a bug introduced by this patch, but looking at the error handling
paths for sun6i_dma_prep_slave_sg() and sun6i_dma_prep_dma_cyclic(), is there
a use-after-free when freeing DMA pool items?

In the err_lli_free path, the code iterates over the linked list of items:

err_lli_free:
	for (p_lli = txd->p_lli, v_lli = txd->v_lli; v_lli;
	     p_lli = v_lli->p_lli_next, v_lli = v_lli->v_lli_next)
		dma_pool_free(sdev->pool, v_lli, p_lli);

When dma_pool_free() executes, the memory associated with v_lli is returned
to the pool. The loop's update step then accesses v_lli->p_lli_next and
v_lli->v_lli_next to advance the pointers.

Under memory pressure, another thread might allocate this newly freed item
and overwrite its contents, potentially causing an invalid pointer
dereference. Should this loop use a safe iteration method that reads the
next pointers before calling dma_pool_free()?
Frank Li June 19, 2026, 4:02 p.m. UTC | #2 
On Fri, Jun 19, 2026 at 04:53:31AM +0000, Yuanshen Cao wrote:
>
> The A733 DMA controller supports higher address (up to 32G) compared to
> previous generations. The existing `sun6i_dma_set_addr` function uses a
> hardcoded logic for setting the high-address bits in the LLI parameters.
>
> By moving `set_addr` into the `sun6i_dma_config` structure, we can
> provide specialized implementations for different hardware. This allows
> the A733 to use a version of `set_addr` that correctly handles its
> specific `SRC_HIGH_ADDR_32G` and `DST_HIGH_ADDR_32G` in the `set_addr`
> register later in the series.
>
> Changes:
> - Added `set_addr` function pointer to `struct sun6i_dma_config`.
> - Refactored `sun6i_dma_set_addr` and introduced
>   `sun6i_dma_set_addr_a100` (keeping the logic for high address
>   support).
> - Updated all existing configuration structs to include the new
>   `set_addr` pointer.
> - Removed `has_high_addr` since the logic is replaced by
>   `sun6i_dma_set_addr_a100`.
>
> Signed-off-by: Yuanshen Cao <alex.caoys@gmail.com>
> ---
>  drivers/dma/sun6i-dma.c | 36 ++++++++++++++++++++++++++----------
>  1 file changed, 26 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index d92e702320d9..059455425e19 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -112,6 +112,7 @@
>
>  /* forward declaration */
>  struct sun6i_dma_dev;
> +struct sun6i_dma_lli;
>
>  /*
>   * Hardware channels / ports representation
> @@ -138,6 +139,8 @@ struct sun6i_dma_config {
>         void (*set_burst_length)(u32 *p_cfg, s8 src_burst, s8 dst_burst);
>         void (*set_drq)(u32 *p_cfg, s8 src_drq, s8 dst_drq);
>         void (*set_mode)(u32 *p_cfg, s8 src_mode, s8 dst_mode);
> +       void (*set_addr)(struct sun6i_dma_dev *sdev, struct sun6i_dma_lli *v_lli,
> +               dma_addr_t src, dma_addr_t dst);
>         void (*dump_com_regs)(struct sun6i_dma_dev *sdev);
>         u32 (*read_irq_en)(struct sun6i_dma_dev *sdev, u32 chan_num);
>         void (*write_irq_en)(struct sun6i_dma_dev *sdev, u32 chan_num, u32 irq_val);
> @@ -147,7 +150,6 @@ struct sun6i_dma_config {
>         u32 dst_burst_lengths;
>         u32 src_addr_widths;
>         u32 dst_addr_widths;
> -       bool has_high_addr;
>         bool has_mbus_clk;
>  };
>
> @@ -675,13 +677,20 @@ static int set_config(struct sun6i_dma_dev *sdev,
>  static inline void sun6i_dma_set_addr(struct sun6i_dma_dev *sdev,
>                                       struct sun6i_dma_lli *v_lli,
>                                       dma_addr_t src, dma_addr_t dst)
> +{
> +       v_lli->src = lower_32_bits(src);
> +       v_lli->dst = lower_32_bits(dst);
> +}
> +
> +static inline void sun6i_dma_set_addr_a100(struct sun6i_dma_dev *sdev,
> +                                     struct sun6i_dma_lli *v_lli,
> +                                     dma_addr_t src, dma_addr_t dst)
>  {
>         v_lli->src = lower_32_bits(src);
>         v_lli->dst = lower_32_bits(dst);
>
> -       if (sdev->cfg->has_high_addr)
> -               v_lli->para |= SRC_HIGH_ADDR(upper_32_bits(src)) |
> -                              DST_HIGH_ADDR(upper_32_bits(dst));
> +       v_lli->para |= SRC_HIGH_ADDR(upper_32_bits(src)) |
> +                               DST_HIGH_ADDR(upper_32_bits(dst));
>  }
>
>  static struct dma_async_tx_descriptor *sun6i_dma_prep_dma_memcpy(
> @@ -714,7 +723,7 @@ static struct dma_async_tx_descriptor *sun6i_dma_prep_dma_memcpy(
>
>         v_lli->len = len;
>         v_lli->para = NORMAL_WAIT;
> -       sun6i_dma_set_addr(sdev, v_lli, src, dest);
> +       sdev->cfg->set_addr(sdev, v_lli, src, dest);

can you move sdev->cfg->set_addr into helper function  sun6i_dma_set_addr())
so need't change other place.

Old sun6i_dma_set_addr() rename sun6i_dma_set_addr_<name>()

sun6i_dma_set_addr()
{
	sdev->cfg->set_addr(sdev, v_lli, src, dest)
}

Frank
diff mbox series

Patch

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index d92e702320d9..059455425e19 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -112,6 +112,7 @@ 
 
 /* forward declaration */
 struct sun6i_dma_dev;
+struct sun6i_dma_lli;
 
 /*
  * Hardware channels / ports representation
@@ -138,6 +139,8 @@  struct sun6i_dma_config {
 	void (*set_burst_length)(u32 *p_cfg, s8 src_burst, s8 dst_burst);
 	void (*set_drq)(u32 *p_cfg, s8 src_drq, s8 dst_drq);
 	void (*set_mode)(u32 *p_cfg, s8 src_mode, s8 dst_mode);
+	void (*set_addr)(struct sun6i_dma_dev *sdev, struct sun6i_dma_lli *v_lli,
+		dma_addr_t src, dma_addr_t dst);
 	void (*dump_com_regs)(struct sun6i_dma_dev *sdev);
 	u32 (*read_irq_en)(struct sun6i_dma_dev *sdev, u32 chan_num);
 	void (*write_irq_en)(struct sun6i_dma_dev *sdev, u32 chan_num, u32 irq_val);
@@ -147,7 +150,6 @@  struct sun6i_dma_config {
 	u32 dst_burst_lengths;
 	u32 src_addr_widths;
 	u32 dst_addr_widths;
-	bool has_high_addr;
 	bool has_mbus_clk;
 };
 
@@ -675,13 +677,20 @@  static int set_config(struct sun6i_dma_dev *sdev,
 static inline void sun6i_dma_set_addr(struct sun6i_dma_dev *sdev,
 				      struct sun6i_dma_lli *v_lli,
 				      dma_addr_t src, dma_addr_t dst)
+{
+	v_lli->src = lower_32_bits(src);
+	v_lli->dst = lower_32_bits(dst);
+}
+
+static inline void sun6i_dma_set_addr_a100(struct sun6i_dma_dev *sdev,
+				      struct sun6i_dma_lli *v_lli,
+				      dma_addr_t src, dma_addr_t dst)
 {
 	v_lli->src = lower_32_bits(src);
 	v_lli->dst = lower_32_bits(dst);
 
-	if (sdev->cfg->has_high_addr)
-		v_lli->para |= SRC_HIGH_ADDR(upper_32_bits(src)) |
-			       DST_HIGH_ADDR(upper_32_bits(dst));
+	v_lli->para |= SRC_HIGH_ADDR(upper_32_bits(src)) |
+				DST_HIGH_ADDR(upper_32_bits(dst));
 }
 
 static struct dma_async_tx_descriptor *sun6i_dma_prep_dma_memcpy(
@@ -714,7 +723,7 @@  static struct dma_async_tx_descriptor *sun6i_dma_prep_dma_memcpy(
 
 	v_lli->len = len;
 	v_lli->para = NORMAL_WAIT;
-	sun6i_dma_set_addr(sdev, v_lli, src, dest);
+	sdev->cfg->set_addr(sdev, v_lli, src, dest);
 
 	burst = convert_burst(8);
 	width = convert_buswidth(DMA_SLAVE_BUSWIDTH_4_BYTES);
@@ -773,7 +782,7 @@  static struct dma_async_tx_descriptor *sun6i_dma_prep_slave_sg(
 		v_lli->para = NORMAL_WAIT;
 
 		if (dir == DMA_MEM_TO_DEV) {
-			sun6i_dma_set_addr(sdev, v_lli,
+			sdev->cfg->set_addr(sdev, v_lli,
 					   sg_dma_address(sg),
 					   sconfig->dst_addr);
 			v_lli->cfg = lli_cfg;
@@ -787,7 +796,7 @@  static struct dma_async_tx_descriptor *sun6i_dma_prep_slave_sg(
 				sg_dma_len(sg), flags);
 
 		} else {
-			sun6i_dma_set_addr(sdev, v_lli,
+			sdev->cfg->set_addr(sdev, v_lli,
 					   sconfig->src_addr,
 					   sg_dma_address(sg));
 			v_lli->cfg = lli_cfg;
@@ -858,7 +867,7 @@  static struct dma_async_tx_descriptor *sun6i_dma_prep_dma_cyclic(
 		v_lli->para = NORMAL_WAIT;
 
 		if (dir == DMA_MEM_TO_DEV) {
-			sun6i_dma_set_addr(sdev, v_lli,
+			sdev->cfg->set_addr(sdev, v_lli,
 					   buf_addr + period_len * i,
 					   sconfig->dst_addr);
 			v_lli->cfg = lli_cfg;
@@ -870,7 +879,7 @@  static struct dma_async_tx_descriptor *sun6i_dma_prep_dma_cyclic(
 				&sconfig->dst_addr, &buf_addr,
 				buf_len, flags);
 		} else {
-			sun6i_dma_set_addr(sdev, v_lli,
+			sdev->cfg->set_addr(sdev, v_lli,
 					   sconfig->src_addr,
 					   buf_addr + period_len * i);
 			v_lli->cfg = lli_cfg;
@@ -1148,6 +1157,7 @@  static struct sun6i_dma_config sun6i_a31_dma_cfg = {
 	.set_burst_length = sun6i_set_burst_length_a31,
 	.set_drq          = sun6i_set_drq_a31,
 	.set_mode         = sun6i_set_mode_a31,
+	.set_addr         = sun6i_dma_set_addr,
 	.dump_com_regs    = sun6i_dma_dump_com_regs,
 	.read_irq_en      = sun6i_read_irq_en,
 	.write_irq_en     = sun6i_write_irq_en,
@@ -1176,6 +1186,7 @@  static struct sun6i_dma_config sun8i_a23_dma_cfg = {
 	.set_burst_length = sun6i_set_burst_length_a31,
 	.set_drq          = sun6i_set_drq_a31,
 	.set_mode         = sun6i_set_mode_a31,
+	.set_addr         = sun6i_dma_set_addr,
 	.dump_com_regs    = sun6i_dma_dump_com_regs,
 	.read_irq_en      = sun6i_read_irq_en,
 	.write_irq_en     = sun6i_write_irq_en,
@@ -1199,6 +1210,7 @@  static struct sun6i_dma_config sun8i_a83t_dma_cfg = {
 	.set_burst_length = sun6i_set_burst_length_a31,
 	.set_drq          = sun6i_set_drq_a31,
 	.set_mode         = sun6i_set_mode_a31,
+	.set_addr         = sun6i_dma_set_addr,
 	.dump_com_regs    = sun6i_dma_dump_com_regs,
 	.read_irq_en      = sun6i_read_irq_en,
 	.write_irq_en     = sun6i_write_irq_en,
@@ -1229,6 +1241,7 @@  static struct sun6i_dma_config sun8i_h3_dma_cfg = {
 	.set_burst_length = sun6i_set_burst_length_h3,
 	.set_drq          = sun6i_set_drq_a31,
 	.set_mode         = sun6i_set_mode_a31,
+	.set_addr         = sun6i_dma_set_addr,
 	.dump_com_regs    = sun6i_dma_dump_com_regs,
 	.read_irq_en      = sun6i_read_irq_en,
 	.write_irq_en     = sun6i_write_irq_en,
@@ -1255,6 +1268,7 @@  static struct sun6i_dma_config sun50i_a64_dma_cfg = {
 	.set_burst_length = sun6i_set_burst_length_h3,
 	.set_drq          = sun6i_set_drq_a31,
 	.set_mode         = sun6i_set_mode_a31,
+	.set_addr         = sun6i_dma_set_addr,
 	.dump_com_regs    = sun6i_dma_dump_com_regs,
 	.read_irq_en      = sun6i_read_irq_en,
 	.write_irq_en     = sun6i_write_irq_en,
@@ -1281,6 +1295,7 @@  static struct sun6i_dma_config sun50i_a100_dma_cfg = {
 	.set_burst_length = sun6i_set_burst_length_h3,
 	.set_drq          = sun6i_set_drq_h6,
 	.set_mode         = sun6i_set_mode_h6,
+	.set_addr         = sun6i_dma_set_addr_a100,
 	.dump_com_regs    = sun6i_dma_dump_com_regs,
 	.read_irq_en      = sun6i_read_irq_en,
 	.write_irq_en     = sun6i_write_irq_en,
@@ -1296,7 +1311,6 @@  static struct sun6i_dma_config sun50i_a100_dma_cfg = {
 			     BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
 			     BIT(DMA_SLAVE_BUSWIDTH_4_BYTES) |
 			     BIT(DMA_SLAVE_BUSWIDTH_8_BYTES),
-	.has_high_addr = true,
 	.has_mbus_clk = true,
 };
 
@@ -1309,6 +1323,7 @@  static struct sun6i_dma_config sun50i_h6_dma_cfg = {
 	.set_burst_length = sun6i_set_burst_length_h3,
 	.set_drq          = sun6i_set_drq_h6,
 	.set_mode         = sun6i_set_mode_h6,
+	.set_addr         = sun6i_dma_set_addr,
 	.dump_com_regs    = sun6i_dma_dump_com_regs,
 	.read_irq_en      = sun6i_read_irq_en,
 	.write_irq_en     = sun6i_write_irq_en,
@@ -1340,6 +1355,7 @@  static struct sun6i_dma_config sun8i_v3s_dma_cfg = {
 	.set_burst_length = sun6i_set_burst_length_a31,
 	.set_drq          = sun6i_set_drq_a31,
 	.set_mode         = sun6i_set_mode_a31,
+	.set_addr         = sun6i_dma_set_addr,
 	.dump_com_regs    = sun6i_dma_dump_com_regs,
 	.read_irq_en      = sun6i_read_irq_en,
 	.write_irq_en     = sun6i_write_irq_en,