Re: [PATCH v2 5/5] NTB: ntb_hw_epf: Discover vNTB-embedded DMA

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Unaligned MMIO offset provided by untrusted device causes alignment fault and kernel panic.
- [Medium] Unsupported DMA extensions or insufficient IRQs cause the core NTB device probe to fail entirely, rather than gracefully falling back to a non-DMA mode.
- [Low] The macro or enum value DW_EDMA_CHIP_PARTIAL is not declared in the kernel tree.
--

commit 47ea035773d643574d3bec0ab584f66f7879214f
Author: Koichiro Den <den@valinux.co.jp>

NTB: ntb_hw_epf: Discover vNTB-embedded DMA

This commit allows a vNTB PCI function to expose endpoint-local DMA channels
to its host. It decodes the extension, registers advertised channels, and
dispatches on the advertised DMA type.

Link: https://lore.kernel.org/r/20260312165005.1148676-1-den@valinux.co.jp/
Link: https://lore.kernel.org/r/20260813063757.3131865-9-den@valinux.co.jp/

> diff --git a/drivers/ntb/hw/epf/ntb_hw_epf.c b/drivers/ntb/hw/epf/ntb_hw_epf.c
> --- a/drivers/ntb/hw/epf/ntb_hw_epf.c
> +++ b/drivers/ntb/hw/epf/ntb_hw_epf.c
[ ... ]
> +static bool ntb_epf_dma_region_parse(struct ntb_epf_dev *ndev, u32 base,
> +				     bool optional,
> +				     struct ntb_epf_dma_region *region)
> +{
> +	resource_size_t bar_len;
> +	u32 bar, offset, size;
> +
> +	bar = readl(ndev->ctrl_reg + NTB_EPF_DMA_REGION_BAR(base));
> +	offset = readl(ndev->ctrl_reg + NTB_EPF_DMA_REGION_OFFSET(base));

[Severity: High]
Is offset validated for alignment here? Since this is read directly from
the untrusted device configuration space, a malicious or buggy endpoint could
provide an unaligned offset.

[ ... ]
> +static int ntb_epf_dw_edma_map_region(struct pci_dev *pdev,
> +				      struct ntb_epf_dma_region *region)
> +{
> +	region->vaddr = pci_iomap_range(pdev, region->bar, region->offset,
> +					region->size);

[Severity: High]
If the region->offset provided by the endpoint is unaligned, does mapping
it via pci_iomap_range() yield an unaligned virtual address?

The dw_edma driver subsequently uses 32-bit accessors (readl) on this
mapping, which can cause an alignment fault (synchronous external abort) and
kernel panic.

[ ... ]
> +static int ntb_epf_dw_edma_init(struct ntb_epf_dev *ndev)
> +{
[ ... ]
> +	chip->dev = ndev->dev;
> +	chip->ops = &ntb_epf_dw_edma_ops;
> +	chip->flags = DW_EDMA_CHIP_PARTIAL;

[Severity: Low]
Does this code build? It looks like DW_EDMA_CHIP_PARTIAL is not declared
in the kernel tree. This might be a leftover from an unmerged or modified
prerequisite patch series.

[ ... ]
> +static int ntb_epf_dma_parse(struct ntb_epf_dev *ndev)
> +{
[ ... ]
> +	switch (type) {
> +	case NTB_EPF_DMA_TYPE_DW_EDMA:
> +		ret = ntb_epf_dw_edma_parse(ndev, length);
> +		break;
> +	default:
> +		return -EOPNOTSUPP;
> +	}

[Severity: Medium]
If the endpoint exposes a newer or unrecognized DMA type, this returns
-EOPNOTSUPP and causes the core NTB device probe to fail entirely.

Would it be better to gracefully fall back to a non-DMA mode so core NTB
functionality (doorbells, memory windows) remains available?

[ ... ]
>  	int i;
>  
> -	irq = pci_alloc_irq_vectors(pdev, msi_min, msi_max, PCI_IRQ_MSIX);
> +	irq = pci_alloc_irq_vectors(pdev, msi_min + dma_irqs,
> +				    msi_max + dma_irqs, PCI_IRQ_MSIX);

[Severity: Medium]
This now strictly requires dma_irqs to be available. If a constrained host
platform lacks enough MSI/MSI-X vectors to satisfy the msi_min + dma_irqs
requirement, pci_alloc_irq_vectors() will fail.

Can we allow the initialization to proceed without DMA support in this
scenario instead of aborting the whole probe?

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