On Thu, Jun 11, 2026 at 1:25 PM <[email protected]> wrote:
>
> >
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s)
> to consider:
> - [Medium] Unconditional validation of the flag-gated 'ph' parameter can lead
> to spurious -EINVAL errors, breaking the syscall trust boundary.
> - [Medium] VFIO_DEVICE_FEATURE_PROBE falsely reports that
> VFIO_DEVICE_FEATURE_DMA_BUF_TPH is supported even if the hardware lacks TPH
> support.
> --
>
> commit d57637d667ca6dc95def77a0a5bd339d62d4f506
> Author: Zhiping Zhang <[email protected]>
>
> vfio/pci: implement get_tph and DMA_BUF_TPH feature
>
> This commit implements dma-buf get_tph for vfio-pci exported dma-bufs and
> adds the VFIO_DEVICE_FEATURE_DMA_BUF_TPH feature. This allows userspace to
> publish TPH metadata for a VFIO-owned device. It checks completer support
> and protects the metadata under dmabuf->resv.
>
> > diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c
> > b/drivers/vfio/pci/vfio_pci_dmabuf.c
> > index 1a177ce7de546..0a0705c8dbea6 100644
> > --- a/drivers/vfio/pci/vfio_pci_dmabuf.c
> > +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c
>
> [ ... ]
>
> > @@ -333,6 +360,71 @@ int vfio_pci_core_feature_dma_buf(struct
> > vfio_pci_core_device *vdev, u32 flags,
> > return ret;
> > }
> >
> > +int vfio_pci_core_feature_dma_buf_tph(struct vfio_pci_core_device *vdev,
> > + u32 flags,
> > + struct vfio_device_feature_dma_buf_tph
> > __user *arg,
> > + size_t argsz)
> > +{
> > + struct vfio_device_feature_dma_buf_tph set_tph;
> > + struct vfio_pci_dma_buf *priv;
> > + struct dma_buf *dmabuf;
> > + u8 comp;
> > + int ret;
> > +
> > + ret = vfio_check_feature(flags, argsz, VFIO_DEVICE_FEATURE_SET,
> > + sizeof(set_tph));
> > + if (ret != 1)
> > + return ret;
>
> [Severity: Medium]
> vfio_check_feature() returns 0 for VFIO_DEVICE_FEATURE_PROBE, causing an
> early return of success.
>
> Should the hardware capability check (pcie_tph_completer_type) occur before
> checking the feature flags, to avoid falsely reporting that the feature is
> supported on devices that lack TPH?
>
Will fix it with a change like below:
- if (ret != 1)
+ if (ret != 1) {
+ if (ret == 0 &&
+ pcie_tph_completer_type(vdev->pdev) ==
+ PCI_EXP_DEVCAP2_TPH_COMP_NONE)
+ return -EOPNOTSUPP;
return ret;
+ }
> > +
> > + if (copy_from_user(&set_tph, arg, sizeof(set_tph)))
> > + return -EFAULT;
> > +
> > + if (set_tph.flags & ~(VFIO_DMA_BUF_TPH_ST | VFIO_DMA_BUF_TPH_ST_EXT))
> > + return -EINVAL;
> > +
> > + if (set_tph.ph & ~0x3)
> > + return -EINVAL;
>
> [Severity: Medium]
> When userspace clears the TPH metadata by passing flags == 0, is it required
> to zero-fill the unused ph field?
>
Not an issue.
