Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] nvme: missing controller state check allows attaching DMA-BUF to dead PCI device --
commit 6741fe43dc1132ae4f51db272567ac5b56987f17 Author: Pavel Begunkov <[email protected]> nvme-pci: implement dma-buf backed requests This patch implements DMA-BUF backed requests for the NVMe PCI driver. It allows mapping DMA-BUFs on registration and storing them as a PRP list to initialize requests. Attached contexts are tracked in a new list synchronized with a lock. > diff --git a/drivers/nvme/host/pci-dmabuf.c b/drivers/nvme/host/pci-dmabuf.c > new file mode 100644 > index 0000000000000..8dab59d936507 > --- /dev/null > +++ b/drivers/nvme/host/pci-dmabuf.c [ ... ] > +static int nvme_init_io_ctx_locked(struct nvme_ctrl *ctrl, > + struct nvme_dma_buf_list *buf_list, > + struct dma_buf_io_ctx *ctx) > +{ > + struct nvme_dma_buf_io_ctx *nvme_ctx; > + struct dma_buf_attachment *attach; > + > + nvme_ctx = kzalloc_obj(*nvme_ctx); > + if (!nvme_ctx) > + return -ENOMEM; > + > + attach = dma_buf_dynamic_attach(ctx->dmabuf, ctrl->dev, > + &nvme_dmabuf_importer_ops, ctx); [Severity: High] Does this code allow attaching a DMA-BUF to a tearing-down or dead PCI device? If the PCI device is hot-unplugged or its driver is unbound, nvme_remove() calls nvme_dev_disable() which cleans up existing DMA-BUF contexts via nvme_pci_remove_dmabuf(). Since nvme_get_ctrl() only pins the char device, not the underlying PCI device binding, could an unprivileged user with an open file descriptor to the block device invoke the register buffers ioctl and race with device removal? > + if (IS_ERR(attach)) { > + kfree(nvme_ctx); > + return PTR_ERR(attach); > + } > + > + nvme_get_ctrl(ctrl); > + list_add(&nvme_ctx->list, &buf_list->ctxs); > + nvme_ctx->attach = attach; > + nvme_ctx->base = ctx; > + nvme_ctx->ctrl = ctrl; > + nvme_ctx->buf_list = buf_list; > + ctx->dev_priv = nvme_ctx; > + ctx->dev_ops = &nvme_dma_buf_io_ops; > + return 0; > +} > + > +int __nvme_init_dma_buf_io_ctx(struct nvme_ctrl *ctrl, > + struct nvme_dma_buf_list *buf_list, > + struct dma_buf_io_ctx *ctx) > +{ > + int ret; > + > + mutex_lock(&buf_list->lock); > + ret = nvme_init_io_ctx_locked(ctrl, buf_list, ctx); [Severity: High] Is a controller state check needed here before proceeding with the attachment? Without verifying ctrl->state, this function can successfully add a new attachment to the device's list, mapping it to a PCI device that is no longer managed by the driver. This could lead to IOMMU domain use-after-free or memory leaks. > + mutex_unlock(&buf_list->lock); > + return ret; > +} -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=7
