Re: [PATCH 1/2] fpga: Add dma-buf interface for FPGA programming

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Passing a DMA-mapped dma-buf scatterlist to the FPGA manager core causes double-mapping or CPU access of device memory, triggering kernel panics.
- [High] The FPGA_IOCTL_LOAD_DMA_BUF ioctl lacks a firmware size parameter, causing the full page-aligned dma-buf capacity to be pushed into the FPGA port, corrupting configuration.
--

--- Patch [1]: [PATCH 1/2] fpga: Add dma-buf interface for FPGA programming ---
commit 9a25ee19d33999a56fc7670a1a9dbec98cb947aa
Author: Aravind Thokala <aravind.thokala@amd.com>

fpga: Add dma-buf interface for FPGA programming

This commit adds a module that allows userspace to pass a pre-allocated DMA
buffer directly to the FPGA manager via ioctl. It creates a miscdevice for
each FPGA manager that registers dma-buf support to map and load the image
without an extra memory copy.

> diff --git a/drivers/fpga/fpga-dmabuf.c b/drivers/fpga/fpga-dmabuf.c
> new file mode 100644
> index 0000000000000..0e92eb1346981
> --- /dev/null
> +++ b/drivers/fpga/fpga-dmabuf.c

[ ... ]

> +static int fpga_dmabuf_load(struct fpga_manager *mgr, int buffd)
> +{
> +	struct fpga_image_info *info;
> +	struct dma_buf_attachment *attach;
> +	struct dma_buf *dmabuf;
> +	struct sg_table *sgt;
> +	int ret;

[ ... ]

> +	sgt = dma_buf_map_attachment_unlocked(attach, DMA_TO_DEVICE);
> +	if (IS_ERR(sgt)) {
> +		ret = PTR_ERR(sgt);
> +		goto err_detach;
> +	}
> +
> +	info->sgt = sgt;
> +
> +	ret = fpga_mgr_lock(mgr);
> +	if (ret)
> +		goto err_unmap;
> +
> +	ret = fpga_mgr_load(mgr, info);

[Severity: Critical]
Does passing a DMA-mapped sg_table into fpga_mgr_load() cause issues with the
underlying FPGA manager drivers?

Hardware drivers typically expect an unmapped CPU sg_table and will call
dma_map_sgtable() on it. Calling dma_map_sgtable() on an already mapped
sg_table can overwrite DMA addresses and corrupt the DMA state.

Additionally, the core manager may fall back to CPU PIO to read the image
header via sg_miter (which internally uses kmap). If the dma-buf is backed
by P2P device memory, this could dereference invalid struct pages and cause
a kernel panic. It also bypasses cache coherency since
dma_buf_begin_cpu_access() is never called.

[ ... ]

> diff --git a/include/uapi/linux/fpga.h b/include/uapi/linux/fpga.h
> new file mode 100644
> index 0000000000000..868650c66526b
> --- /dev/null
> +++ b/include/uapi/linux/fpga.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +/*
> + * FPGA userspace API
> + *
> + * Copyright (C) 2026 Advanced Micro Devices, Inc.
> + */
> +#ifndef _UAPI_LINUX_FPGA_H
> +#define _UAPI_LINUX_FPGA_H
> +
> +#include <linux/ioctl.h>
> +#include <linux/types.h>
> +
> +#define FPGA_IOCTL_LOAD_DMA_BUF	_IOW('J', 1, __s32)

[Severity: High]
Since this IOCTL only takes a file descriptor (__s32), how does userspace
supply the exact payload size of the firmware?

In fpga_dmabuf_load(), the info struct is zero-allocated, leaving
info->data_size as 0. When fpga_mgr_buf_load_sg() evaluates the scatterlist,
if data_size is 0, it loops over the entire mapped capacity of the sg_table.

Because dma-bufs are typically page-aligned, will this result in writing
padding bytes at the end of the buffer directly into the FPGA configuration
port, potentially corrupting the hardware state?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260816150734.2092802-1-aravind.thokala@amd.com?part=1