Re: [PATCH 2/2] fpga: versal: add dma-buf programming support

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Passing a user-accessible dma-buf directly to the PMC firmware introduces a speculative Time-of-Check to Time-of-Use (TOCTOU) vulnerability that can potentially bypass FPGA bitstream authentication.
--

--- Patch [2]: [PATCH 2/2] fpga: versal: add dma-buf programming support ---
commit 92384704c37fbbfe16cd9659151fbb8646c178cc
Author: Aravind Thokala <[email protected]>

fpga: versal: add dma-buf programming support

This commit adds a .write_sg callback to the Versal FPGA driver to support
dma-buf based programming. It extracts the DMA address from the scatter-gather
table and passes it directly to the firmware, avoiding an intermediate buffer
copy.

> diff --git a/drivers/fpga/versal-fpga.c b/drivers/fpga/versal-fpga.c
> index e6189106c4686..bdaa15a6c9913 100644
> --- a/drivers/fpga/versal-fpga.c
> +++ b/drivers/fpga/versal-fpga.c

[ ... ]

> @@ -37,11 +38,34 @@ static int versal_fpga_ops_write(struct fpga_manager *mgr,
>  	return ret;
>  }
>  
> +static int versal_fpga_ops_write_sg(struct fpga_manager *mgr,
> +				    struct sg_table *sgt)
> +{
> +	dma_addr_t dma_addr;
> +
> +	/* zynqmp_pm_load_pdi() takes a single base address */
> +	if (sgt->nents != 1) {
> +		dev_err(&mgr->dev, "dma-buf has %u segments, need exactly 1\n",
> +			sgt->nents);
> +		return -EINVAL;
> +	}
> +
> +	dma_addr = sg_dma_address(sgt->sgl);
> +
> +	return zynqmp_pm_load_pdi(PDI_SRC_DDR, dma_addr);

[Severity: Medium]
Does this expose the firmware to a Time-of-Check to Time-of-Use (TOCTOU)
vulnerability if the Xilinx PMC firmware double-fetches the payload?

By extracting the DMA address from the user-provided dma-buf and passing
it directly to zynqmp_pm_load_pdi() without copying it to a kernel-owned
isolated buffer, a process could potentially modify the bitstream
concurrently during the firmware's authentication and loading process.

If a user with CAP_SYS_RAWIO maintains a writable memory mapping to the
dma-buf (for instance, using udmabuf) and invokes FPGA_IOCTL_LOAD_DMA_BUF,
they could overwrite the memory while the firmware reads it.

If the firmware re-reads any part of the bitstream after authentication,
could this be abused to bypass secure boot boundaries?

> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2