Re: [PATCH 2/4] dmaengine: dw-edma-pcie: Reject devices without driver data

[sashiko-bot@kernel.org]

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

New issues:
- [Medium] Mixing goto-based error handling with scope-based cleanup (__free) in the same function.

Pre-existing issues:
- [High] Missing bounds check for the hardware-provided BAR index when accessing the `pcim_iomap_table()` array.
--

commit 451669b3f83e6d7ef131e591fd15b9e144ab9005
Author: Koichiro Den <den@valinux.co.jp>
Subject: dmaengine: dw-edma-pcie: Reject devices without driver data

This commit adds a check to reject devices without driver data during
dw_edma_pcie_probe. This prevents a NULL pointer dereference when a device
bound dynamically via sysfs matches the driver without that data.

> diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
> index 87c31d01fb102..c2024fa824e03 100644
> --- a/drivers/dma/dw-edma/dw-edma-pcie.c
> +++ b/drivers/dma/dw-edma/dw-edma-pcie.c
> @@ -314,6 +314,9 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
>  	int i, mask;
>  	bool non_ll = false;
>  
> +	if (!pdata)
> +		return -ENODEV;
> +
>  	struct dw_edma_pcie_data *vsec_data __free(kfree) =
>  		kmalloc_obj(*vsec_data);

[Severity: Medium]
The kernel's cleanup subsystem guidelines state that goto-based error
handling and scope-based cleanup should not be mixed in the same function.

Since dw_edma_pcie_probe() uses goto-based cleanup labels on failure paths
like goto err_free_irq_vectors, could mixing them with the __free attribute
here lead to confusing ownership semantics or resource tracking bugs?

Would it be better to either convert all cleanups in this function to
scope-based guards, or avoid using the __free attribute entirely?

[ ... ]

>  	chip->ll_wr_cnt = vsec_data->wr_ch_cnt;
>  	chip->ll_rd_cnt = vsec_data->rd_ch_cnt;
>  
>  	chip->reg_base = pcim_iomap_table(pdev)[vsec_data->rg.bar];

[Severity: High]
This is a pre-existing issue, but does this code properly validate the
hardware-provided BAR index before using it as an array index?

The value for vsec_data->rg.bar is extracted from the PCIe VSEC
capabilities, which can yield values up to 7. However,
pcim_iomap_table(pdev) returns a pointer to an array of size
PCI_STD_NUM_BARS, which is 6.

If a misconfigured or malicious device provides a BAR index of 6 or 7,
could accessing this array result in an out-of-bounds memory read and
crash the kernel when the resulting invalid pointer is dereferenced?

>  	if (!chip->reg_base) {
>  		err = -ENOMEM;
>  		goto err_free_irq_vectors;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260521142153.2957432-1-den@valinux.co.jp?part=2