Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [Low] The function `vfio_pci_core_feature_dma_buf_revoke` mixes `goto`-based 
error handling with scope-based cleanup helpers (`scoped_guard`).
--

--- Patch [9]: [PATCH v5 9/9] vfio/pci: Permanently revoke a DMABUF on request 
---
commit 668793566cfbdb5cee2b0a69188ebbada38dd4d5
Author: Matt Evans <[email protected]>

vfio/pci: Permanently revoke a DMABUF on request

This commit expands the VFIO DMABUF revocation state to allow permanent
revocation via a new VFIO_DEVICE_FEATURE_DMA_BUF_REVOKE feature request.
It refactors the revocation logic into a common function used by both
the move path and the new feature request path.

> diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c 
> b/drivers/vfio/pci/vfio_pci_dmabuf.c
> index ccfe861f88b66..36bf07530840d 100644
> --- a/drivers/vfio/pci/vfio_pci_dmabuf.c
> +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c
[ ... ]
> @@ -671,9 +696,72 @@ void vfio_pci_dma_buf_cleanup(struct 
> vfio_pci_core_device *vdev)
[ ... ]
> +     if (dmabuf->ops != &vfio_pci_dmabuf_ops ||
> +         READ_ONCE(priv->vdev) != vdev) {
> +             ret = -ENODEV;
> +             goto out_put_buf;
> +     }
> +
> +     scoped_guard(rwsem_write, &vdev->memory_lock) {

[Severity: Low]
Is it intentional to mix goto-based error handling and scope-based cleanup
helpers in the same function?

The subsystem guidelines for cleanup helpers recommend against mixing goto
labels with scoped_guard in the same routine, to avoid confusing ownership
semantics and potential resource leaks during future refactoring.

Could the dma_buf_put cleanup at the out_put_buf label be converted to use a
scope-based helper, or could the scoped_guard be replaced with traditional
locking, to keep the error handling style consistent throughout
vfio_pci_core_feature_dma_buf_revoke?

> +             if (priv->status == VFIO_PCI_DMABUF_PERM_REVOKED) {
> +                     ret = -EBADFD;
> +             } else {
> +                     vfio_pci_dma_buf_set_status(priv,
> +                                                 
> VFIO_PCI_DMABUF_PERM_REVOKED);
> +                     ret = 0;
> +             }
> +     }
> +
> +out_put_buf:
> +     dma_buf_put(dmabuf);
> +
> +     return ret;
> +}
> +#endif /* CONFIG_VFIO_PCI_DMABUF */

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=9

Reply via email to