Re: [PATCH] net: tun: fix tun_xdp_one() for IFF_TUN mode

[Jason Wang <jasowang@redhat.com>]

在 2021/6/22 下午4:29, David Woodhouse 写道:
> On Tue, 2021-06-22 at 16:00 +0800, Jason Wang wrote:
>>> I'm tempted to add a new feature for that 1:1 access, with no ->umem or
>>> ->iotlb at all. And then I can use it as a key to know that the XDP
>>> bugs are fixed too :)
>>
>> This means we need validate the userspace address each time before vhost
>> tries to use that. This will de-gradate the performance. So we still
>> need to figure out the legal userspace address range which might not be
>> easy.
> Easier from the kernel than from userspace though :)


Yes.


>
> But I don't know that we need it. Isn't a call to access_ok() going to
> be faster than what translate_desc() does to look things up anyway?


Right.


>
> In the 1:1 mode, the access_ok() is all that's needed since there's no
> translation.
>
> @@ -2038,6 +2065,14 @@ static int translate_desc(struct vhost_virtqueue *vq, u64 addr, u32 len,
>          u64 s = 0;
>          int ret = 0;
>   
> +       if (vhost_has_feature(vq, VHOST_F_IDENTITY_MAPPING)) {


Using vhost_has_feature() is kind of tricky since it's used for virtio 
feature negotiation.

We probably need to use backend_features instead.

I think we should probably do more:

1) forbid the feature to be set when mem table / IOTLB has at least one 
mapping
2) forbid the mem table / IOTLB updating after the feature is set

Thanks


> +               if (!access_ok((void __user *)addr, len))
> +                       return -EFAULT;
> +
> +               iov[0].iov_len = len;
> +               iov[0].iov_base = (void __user *)addr;
> +               return 1;
> +       }
>          while ((u64)len > s) {
>                  u64 size;
>                  if (unlikely(ret >= iov_size)) {