Re: [PATCH v11 07/13] nvme-pci: implement dma-buf backed requests

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] nvme: heap buffer overflow in nvme_dma_buf_io_map() on oversized sg_table
--

commit c0ad27ffb27d58a9a9cd8c22288b3a3645727e63
Author: Pavel Begunkov <asml.silence@gmail.com>

nvme-pci: implement dma-buf backed requests
    
This commit adds support for DMA-buf backed requests in the NVMe PCI host
driver. On registration, the dma-buf is mapped and stored as a prp list to
initialize requests. All attached contexts are stored in a new list
synchronized with a mutex.

> diff --git a/drivers/nvme/host/pci-dmabuf.c b/drivers/nvme/host/pci-dmabuf.c
> new file mode 100644
> index 0000000000000..161e4ffa6919b
> --- /dev/null
> +++ b/drivers/nvme/host/pci-dmabuf.c
[ ... ]
> +static struct dma_buf_io_map *nvme_dma_buf_io_map(struct dma_buf_io_ctx *ctx)
> +{
> +	unsigned nr_entries = ctx->dmabuf->size / NVME_CTRL_PAGE_SIZE;
> +	struct nvme_dma_buf_io_ctx *nvme_ctx = ctx->dev_priv;
> +	struct dma_buf_attachment *attach = nvme_ctx->attach;
> +	unsigned long tmp, i = 0;
> +	struct nvme_dmabuf_map *map;
> +	struct scatterlist *sg;
> +	struct sg_table *sgt;
> +	int ret;
> +
> +	dma_resv_assert_held(ctx->dmabuf->resv);
> +
> +	if (ctx->dmabuf->size % NVME_CTRL_PAGE_SIZE)
> +		return ERR_PTR(-EINVAL);
> +
> +	map = kvmalloc_flex(*map, dma_list, nr_entries);
> +	if (!map)
> +		return ERR_PTR(-ENOMEM);
> +	map->nr_entries = nr_entries;
> +
> +	sgt = dma_buf_map_attachment(attach, ctx->dir);
> +	if (IS_ERR(sgt)) {
> +		ret = PTR_ERR(sgt);
> +		sgt = NULL;
> +		goto err;
> +	}
> +
> +	for_each_sgtable_dma_sg(sgt, sg, tmp) {
> +		dma_addr_t dma_addr = sg_dma_address(sg);
> +		unsigned long sg_len = sg_dma_len(sg);
> +
> +		if ((sg_len % NVME_CTRL_PAGE_SIZE) ||
> +		    (dma_addr % NVME_CTRL_PAGE_SIZE)) {
> +			ret = -EINVAL;
> +			goto err;
> +		}
> +		while (sg_len) {
> +			map->dma_list[i++] = dma_addr;

[Severity: Critical]
Could this lead to a heap buffer overflow?

The flex array map->dma_list is allocated with a fixed capacity of
nr_entries, which is derived strictly from ctx->dmabuf->size.
However, this loop populates the array based on the total length of the
sg_table returned by dma_buf_map_attachment().

Because dma_buf_map_attachment() does not enforce that the returned
sg_table's length matches dmabuf->size, a buggy or malicious exporter
could return a larger table. In that case, i would exceed nr_entries
without any bounds check, leading to an out-of-bounds write. This
could allow memory corruption triggerable by unprivileged users capable
of creating and exporting dmabufs.

> +			dma_addr += NVME_CTRL_PAGE_SIZE;
> +			sg_len -= NVME_CTRL_PAGE_SIZE;
> +		}
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1791975048.git.asml.silence@gmail.com?part=7