Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] RDMA/uverbs: use-after-free in uverbs_dmabuf_match_mapping() after unbind --
commit 573a52ef0d548646c203780dd26bd3aa8dbc3284 Author: Jason Gunthorpe <[email protected]> RDMA/uverbs: Use the SGT_P2P mapping type This commit converts the RDMA uverbs dma-buf exporter to use the SGT_P2P mapping type via uverbs_dmabuf_match_mapping() and removes the uverbs_dmabuf_attach() callback. > diff --git a/drivers/infiniband/core/uverbs_std_types_dmabuf.c > b/drivers/infiniband/core/uverbs_std_types_dmabuf.c > index 2411ebee69e2d..3c5c3ea2cf556 100644 > --- a/drivers/infiniband/core/uverbs_std_types_dmabuf.c > +++ b/drivers/infiniband/core/uverbs_std_types_dmabuf.c [ ... ] > @@ -76,13 +67,30 @@ static void uverbs_dmabuf_release(struct dma_buf *dmabuf) > uverbs_uobject_release(&priv->uobj); > } > > -static const struct dma_buf_ops uverbs_dmabuf_ops = { > - .attach = uverbs_dmabuf_attach, > +static const struct dma_buf_mapping_sgt_exp_ops uverbs_dma_buf_sgt_ops = { > .map_dma_buf = uverbs_dmabuf_map, > .unmap_dma_buf = uverbs_dmabuf_unmap, > +}; > + > +static int uverbs_dmabuf_match_mapping(struct dma_buf_match_args *args) > +{ > + struct ib_uverbs_dmabuf_file *priv = args->dmabuf->priv; > + struct dma_buf_mapping_match sgt_match[1]; > + > + if (WARN_ON(!dev_is_pci(priv->provider->owner))) > + return -EINVAL; > + > + sgt_match[0] = DMA_BUF_EMAPPING_SGT_P2P( > + &uverbs_dma_buf_sgt_ops, to_pci_dev(priv->provider->owner)); > + > + return dma_buf_match_mapping(args, sgt_match, ARRAY_SIZE(sgt_match)); > +} [Severity: High] Can uverbs_dmabuf_match_mapping() cause a use-after-free of priv->provider and priv->provider->owner if an importer attaches to the dma-buf after the exporting PCI/RDMA driver unbinds or the PCI device is removed? During UVERBS_METHOD_DMABUF_ALLOC, priv->provider is populated via ib_dev->ops.mmap_get_pfns() -> mlx5_ib_mmap_get_pfns() -> pcim_p2pdma_provider(): drivers/pci/p2pdma.c:pcim_p2pdma_provider() { ... p2p = rcu_dereference_protected(pdev->p2pdma, 1); if (WARN_ON(!p2p)) /* Someone forgot to call to pcim_p2pdma_init() before */ return NULL; return &p2p->mem[bar]; } Because struct pci_p2pdma is allocated with devm_kzalloc(&pdev->dev, ...) in pcim_p2pdma_init(), its lifetime is tied to the PCI driver binding rather than the exported dma-buf file. When the driver is unbound or hot-unplugged while userspace still holds the dma-buf fd open, ib_uverbs_remove_one() calls ib_uverbs_free_hw_resources() -> uverbs_destroy_ufile_hw() -> uverbs_dmabuf_fd_destroy_uobj(): drivers/infiniband/core/uverbs_std_types_dmabuf.c:uverbs_dmabuf_fd_destroy_uobj() { ... dma_resv_lock(uverbs_dmabuf->dmabuf->resv, NULL); if (!uverbs_dmabuf->revoked) { uverbs_dmabuf->revoked = true; list_del(&uverbs_dmabuf->dmabufs_elm); dma_buf_invalidate_mappings(uverbs_dmabuf->dmabuf); dma_resv_wait_timeout(uverbs_dmabuf->dmabuf->resv, DMA_RESV_USAGE_BOOKKEEP, false, MAX_SCHEDULE_TIMEOUT); wait_for_comp = true; } dma_resv_unlock(uverbs_dmabuf->dmabuf->resv); ... } After revoking and waiting for active mappings to unmap, ib_uverbs_remove_one() sets wait_clients = 0 and returns without waiting for open dma-buf fds to close. Once the PCI driver remove completes, devres frees struct pci_p2pdma (leaving priv->provider dangling), and if the PCI device is removed, struct pci_dev (priv->provider->owner) can also be freed since struct ib_uverbs_dmabuf_file does not hold a pci_dev_get() reference. Previously, uverbs_dmabuf_attach() only checked attachment->peer2peer, and priv->provider was only accessed inside uverbs_dmabuf_map() under dma_resv_lock() after checking !priv->revoked. With this change, dma_buf_mapping_attach() invokes uverbs_dmabuf_match_mapping() without holding dmabuf->resv or checking priv->revoked, and unconditionally dereferences priv->provider->owner and passes the pci_dev to dma_buf_match_mapping() -> dma_buf_sgt_match() -> pci_p2pdma_distance(). Notice that commit 9770a3eb805c ("vfio/pci: Change the DMA-buf exporter to use mapping_type") earlier in this series avoided this same issue in vfio_pci_dmabuf.c by storing a reference-counted struct pci_dev pointer in priv->pdev for the lifetime of the dma-buf and passing priv->pdev in match_mapping(). Should uverbs_std_types_dmabuf.c similarly hold a reference or synchronize against revocation here? -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=16
