Re: [PATCH v19 12/18] vfio/pci: Add dmabuf TPH metadata storage and fd query helper

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] vfio_pci_dma_buf_get_tph_by_fd() fails to acquire the dma_resv lock before calling vfio_pci_dma_buf_get_pci_tph(), leading to a guaranteed lockdep splat and a potential data race.
--

commit cb20e399309e26c4cb897d3a87f672443b48caa4
Author: Chengwen Feng <fengchengwen@huawei.com>

vfio/pci: Add dmabuf TPH metadata storage and fd query helper

This commit adds TPH metadata fields to vfio_pci_dma_buf to store
PH and ST information along with their validity flags. It also implements
helpers to fetch this metadata from a dmabuf, providing the foundation
for upcoming TPH resolution features.

> diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c b/drivers/vfio/pci/vfio_pci_dmabuf.c
> index c16f460c01d68..d9c5e434e586f 100644
> --- a/drivers/vfio/pci/vfio_pci_dmabuf.c
> +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c
[ ... ]
> @@ -81,6 +88,26 @@ static void vfio_pci_dma_buf_unmap(struct dma_buf_attachment *attachment,
>  	kref_put(&priv->kref, vfio_pci_dma_buf_done);
>  }
>  
> +static int vfio_pci_dma_buf_get_pci_tph(struct dma_buf *dmabuf, bool extended,
> +					u16 *tag, u8 *ph)
> +{
> +	struct vfio_pci_dma_buf *priv = dmabuf->priv;
> +
> +	dma_resv_assert_held(priv->dmabuf->resv);
> +
> +	if (extended) {
[ ... ]
> @@ -106,6 +133,26 @@ static const struct dma_buf_ops vfio_pci_dmabuf_ops = {
>  	.release = vfio_pci_dma_buf_release,
>  };
>  
> +int vfio_pci_dma_buf_get_tph_by_fd(int fd, bool extended, u16 *tag, u8 *ph)
> +{
> +	struct dma_buf *dmabuf;
> +	int ret = 0;
> +
> +	dmabuf = dma_buf_get(fd);
> +	if (IS_ERR(dmabuf))
> +		return PTR_ERR(dmabuf);
> +
> +	if (dmabuf->ops != &vfio_pci_dmabuf_ops) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	ret = vfio_pci_dma_buf_get_pci_tph(dmabuf, extended, tag, ph);

[Severity: High]
Is it necessary to acquire the dmabuf->resv lock before making this call?

Since vfio_pci_dma_buf_get_pci_tph() asserts the lock is held, it seems
invoking it directly from here will guarantee a lockdep splat.

It also appears this could leave the read of the TPH metadata bitfields
unprotected against concurrent updates, resulting in a data race.

> +out:
> +	dma_buf_put(dmabuf);
> +	return ret;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260702124224.57168-1-fengchengwen@huawei.com?part=12