Re: [PATCH v9 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:
- [High] block: nvme: heap buffer overflow in nvme_dma_buf_io_map() due to missing bounds check
--

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

nvme-pci: implement dma-buf backed requests

This patch enables BIO_DMABUF_MAP backed requests. On registration, it maps
the dma-buf and stores it as a prp list used to initialize requests. All
attached contexts are stored in a new list dmabuf_ctxs, and synchronized
with dmabuf_lock.

> diff --git a/drivers/nvme/host/pci-dmabuf.c b/drivers/nvme/host/pci-dmabuf.c
> new file mode 100644
> index 0000000000000..d58a38a75d673
> --- /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: High]
Does this code overflow the map->dma_list[] flex array if the total mapped
length exceeds the calculated ctx->dmabuf->size?

nvme_dma_buf_io_map() calculates the number of flex array entries (nr_entries)
based strictly on ctx->dmabuf->size. However, the loop that populates
map->dma_list is bounded by the total sg_dma_len(sg) across all entries in the
scatterlist returned by dma_buf_map_attachment().

The DMA mapping API explicitly allows the mapped DMA length (sg_dma_len) to be
larger than the original buffer size. For instance, this can occur when an
IOMMU coalesces entries or aligns sizes to a larger IOMMU page size. A buggy
exporter could also supply a larger-than-expected scatterlist.

Without bounds-checking i against nr_entries, writing to map->dma_list[i++]
could result in a heap buffer overflow.

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

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