Re: [PATCH 3/4] vfio/pci: Add PCIe TPH GET_ST interface

[fengchengwen <fengchengwen@huawei.com>]

On 4/15/2026 9:55 PM, Wathsala Vithanage wrote:
> Hi Feng,
> 
> get_st  feature is unsafe. It allows a rogue userspace driver in device-specific
> mode to obtain steering tags for arbitrary CPUs, including ones unrelated
> to the device or its workload, enabling it to direct traffic into those CPUs’
> caches and potentially interfere with other workloads, opening doors to
> further exploits depending on other vulnerabilities.

Thank you for the follow-up and for referencing the prior RFC
discussion on this topic. I appreciate you clarifying the
historical context of the safety concerns.

I acknowledge the risks you’ve highlighted, but I believe the
risk profile in this VFIO interface is different and already
well bounded by existing design and practice:

1. VFIO device access requires elevated privileges
   A userspace process can only open a VFIO device node if it
   has sufficient privileges (typically root). This is not an
   interface for unprivileged users.

2. In the thread "[RFC v2 0/2] Retrieve tph from dmabuf for PCIe
   P2P memory access", applications can configure the steertag
   of exported dmabufs from userspace to the kernel. Kernel PCIe
   drivers (e.g., mlx5 NIC) then use these steertags and set them
   to their ST tables. Even here, userspace could set invalid
   steertags that impact GPU performance—but this model is
   basically accepted I think (refer from maillist discuss).

3. Malicious resource consumption is not unique to TPH
   A malicious thread can be created to forcibly consume CPU
   resources and bound to a specific CPU, affecting other CPUs.
   This is a general system security concern, not one specific
   to TPH GET_ST, and is addressed by existing system hardening
   and access control mechanisms—not by removing useful features.

4. GET_ST is strictly necessary for Device-Specific (DS) mode
   when no ST table is present on the device.
   For devices that do not have a dedicated ST table (a common
   scenario in many PCIe endpoints), DS mode requires userspace
   to retrieve per-CPU steering tags first, then program them
   into the device’s steering logic via other registers. Without
   GET_ST, userspace cannot obtain the required steertags to
   enable TPH DS mode at all—rendering TPH support useless for
   these devices. This is not an optional feature but a
   fundamental requirement to unlock TPH functionality for a
   large class of hardware.

Given these points—privilege restriction, existing industry
practice, general system security mitigations, and strict
functional necessity for DS mode—I believe GET_ST is reasonable
and consistent with existing VFIO security boundaries.

Thanks

> 
> That's why we dropped this capability in https://lore.kernel.org/kvm/20251013163515.16565-1-wathsala.vithanage@arm.com/
> 
> --wathsala
> 
>>   }
>>   +static int vfio_pci_tph_get_st(struct vfio_pci_core_device *vdev,
>> +                   struct vfio_device_pci_tph_op *op,
>> +                   void __user *uarg)
>> +{
>> +    struct vfio_pci_tph_entry *ents;
>> +    struct vfio_pci_tph_st st;
>> +    enum tph_mem_type mtype;
>> +    size_t size;
>> +    int i, err;
>> +
>> +    if (copy_from_user(&st, uarg, sizeof(st)))
>> +        return -EFAULT;
>> +
>> +    if (!st.count || st.count > 2048)
>> +        return -EINVAL;
>> +
>> +    size = st.count * sizeof(*ents);
>> +    ents = kvmalloc(size, GFP_KERNEL);
>> +    if (!ents)
>> +        return -ENOMEM;
>> +
>> +    if (copy_from_user(ents, uarg + sizeof(st), size)) {
>> +        err = -EFAULT;
>> +        goto out;
>> +    }
>> +
>> +    for (i = 0; i < st.count; i++) {
>> +        if (ents[i].mem_type == VFIO_PCI_TPH_MEM_TYPE_VM) {
>> +            mtype = TPH_MEM_TYPE_VM;
>> +        } else if (ents[i].mem_type == VFIO_PCI_TPH_MEM_TYPE_PM) {
>> +            mtype = TPH_MEM_TYPE_PM;
>> +        } else {
>> +            err = -EINVAL;
>> +            goto out;
>> +        }
>> +
>> +        err = pcie_tph_get_cpu_st(vdev->pdev, mtype, ents[i].cpu, &ents[i].st);
>> +        if (err)
>> +            goto out;
>> +    }
>> +
>> +    if (copy_to_user(uarg + sizeof(st), ents, size))
>> +        err = -EFAULT;
>> +
>> +out:
>> +    kvfree(ents);
>> +    return err;
>> +}
>> +
>>   static int vfio_pci_ioctl_tph(struct vfio_pci_core_device *vdev,
>>                     void __user *uarg)
>>   {
>> @@ -1544,6 +1593,8 @@ static int vfio_pci_ioctl_tph(struct vfio_pci_core_device *vdev,
>>           return vfio_pci_tph_enable(vdev, &op, uarg + minsz);
>>       case VFIO_PCI_TPH_DISABLE:
>>           return vfio_pci_tph_disable(vdev);
>> +    case VFIO_PCI_TPH_GET_ST:
>> +        return vfio_pci_tph_get_st(vdev, &op, uarg + minsz);
>>       default:
>>           /* Other ops are not implemented yet */
>>           return -EINVAL;
>