Re: [PATCH v3 4/4] dmaengine: dw-axi-dmac: Use bitfield helpers for registers

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Kernel crash in axi_desc_put due to dma_pool_free() being called with a NULL virtual address when descriptor setup fails.
--

commit cfefdb6fe5a6c1faad810fe6247a2229e7ffd900
Author: Jia Wang <wangjia@ultrarisc.com>

    dmaengine: dw-axi-dmac: Use bitfield helpers for registers
    
    The driver open-codes several channel configuration and descriptor
    control bitfield writes with left shifts. Define masks for those fields
    and use FIELD_PREP() when programming the registers.
    
    Valid field values keep the same encoding. FIELD_PREP() confines values
    to their respective fields, but does not validate handshake numbers
    supplied through DMA specifiers.

> diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> index 61230d2b1c561..b4aa6461684cb 100644
> --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
[ ... ]
> @@ -977,19 +980,19 @@ dma_chan_prep_dma_memcpy(struct dma_chan *dchan, dma_addr_t dst_adr,
>  		if (chan->chip->dw->hdata->restrict_axi_burst_len) {
>  			u32 burst_len = chan->chip->dw->hdata->axi_rw_burst_len - 1;
>  
> -			reg |= (CH_CTL_H_ARLEN_EN |
> -				burst_len << CH_CTL_H_ARLEN_POS |
> -				CH_CTL_H_AWLEN_EN |
> -				burst_len << CH_CTL_H_AWLEN_POS);
> +			reg |= CH_CTL_H_ARLEN_EN |
> +			       FIELD_PREP(CH_CTL_H_ARLEN, burst_len) |
> +			       CH_CTL_H_AWLEN_EN |
> +			       FIELD_PREP(CH_CTL_H_AWLEN, burst_len);
>  		}
>  		hw_desc->lli->ctl_hi = cpu_to_le32(reg);
>  
> -		reg = (DWAXIDMAC_BURST_TRANS_LEN_4 << CH_CTL_L_DST_MSIZE_POS |
> -		       DWAXIDMAC_BURST_TRANS_LEN_4 << CH_CTL_L_SRC_MSIZE_POS |
> -		       xfer_width << CH_CTL_L_DST_WIDTH_POS |
> -		       xfer_width << CH_CTL_L_SRC_WIDTH_POS |
> -		       DWAXIDMAC_CH_CTL_L_INC << CH_CTL_L_DST_INC_POS |
> -		       DWAXIDMAC_CH_CTL_L_INC << CH_CTL_L_SRC_INC_POS);
> +		reg = FIELD_PREP(CH_CTL_L_DST_MSIZE, DWAXIDMAC_BURST_TRANS_LEN_4) |
> +		      FIELD_PREP(CH_CTL_L_SRC_MSIZE, DWAXIDMAC_BURST_TRANS_LEN_4) |
> +		      FIELD_PREP(CH_CTL_L_DST_WIDTH, xfer_width) |
> +		      FIELD_PREP(CH_CTL_L_SRC_WIDTH, xfer_width) |
> +		      FIELD_PREP(CH_CTL_L_DST_INC, DWAXIDMAC_CH_CTL_L_INC) |
> +		      FIELD_PREP(CH_CTL_L_SRC_INC, DWAXIDMAC_CH_CTL_L_INC);
>  		hw_desc->lli->ctl_lo = cpu_to_le32(reg);
>  
>  		set_desc_src_master(hw_desc);

[Severity: High]
This is a pre-existing issue, but can the error handling path in
dma_chan_prep_dma_memcpy() cause a kernel crash?

If axi_desc_get() fails to allocate a descriptor from the DMA pool, the code
jumps to err_desc_get and calls axi_desc_put(desc):

drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c:dma_chan_prep_dma_memcpy() {
	...
		hw_desc->lli = axi_desc_get(chan, &hw_desc->llp);
		if (unlikely(!hw_desc->lli))
			goto err_desc_get;
	...
err_desc_get:
	if (desc)
		axi_desc_put(desc);
	return NULL;
}

In axi_desc_put(), the loop unconditionally iterates over all requested
hardware descriptors up to desc->nr_hw_descs and attempts to free them:

drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c:axi_desc_put() {
	...
	for (descs_put = 0; descs_put < count; descs_put++) {
		hw_desc = &desc->hw_desc[descs_put];
		dma_pool_free(chan->desc_pool, hw_desc->lli, hw_desc->llp);
	}
	...
}

Because unallocated descriptors will have a NULL hw_desc->lli, dma_pool_free()
is called with a NULL virtual address. Since dma_pool_free() does not check
for a NULL vaddr input, will this trigger a NULL pointer dereference in the
DMA pool allocator?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902-dma-fix-v3-0-414c6449fbcc@ultrarisc.com?part=4