| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CACGkMEtTnwG2y4RT6E7tzkWh0rm2xJJEgV8cvjWkqt3th-4DUA@mail.gmail.com>
Date: Tue, 2 Aug 2022 15:08:22 +0800
From: Jason Wang <jasowang@...hat.com>
To: Yongji Xie <xieyongji@...edance.com>
Cc: mst <mst@...hat.com>, Liu Xiaodong <xiaodong.liu@...el.com>,
Maxime Coquelin <maxime.coquelin@...hat.com>,
Stefan Hajnoczi <stefanha@...hat.com>,
songmuchun@...edance.com,
virtualization <virtualization@...ts.linux-foundation.org>,
linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 5/5] vduse: Support querying information of IOVA regions
On Fri, Jul 29, 2022 at 11:17 AM Yongji Xie <xieyongji@...edance.com> wrote:
>
> On Thu, Jul 28, 2022 at 5:02 PM Jason Wang <jasowang@...hat.com> wrote:
> >
> > On Thu, Jul 28, 2022 at 4:27 PM Yongji Xie <xieyongji@...edance.com> wrote:
> > >
> > > On Thu, Jul 28, 2022 at 2:45 PM Jason Wang <jasowang@...hat.com> wrote:
> > > >
> > > > On Thu, Jul 28, 2022 at 2:36 PM Yongji Xie <xieyongji@...edance.com> wrote:
> > > > >
> > > > > On Thu, Jul 28, 2022 at 1:58 PM Jason Wang <jasowang@...hat.com> wrote:
> > > > > >
> > > > > > On Thu, Jul 28, 2022 at 11:20 AM Xie Yongji <xieyongji@...edance.com> wrote:
> > > > > > >
> > > > > > > This introduces a new ioctl: VDUSE_IOTLB_GET_INFO to
> > > > > > > support querying some information of IOVA regions.
> > > > > > >
> > > > > > > Now it can be used to query whether the IOVA region
> > > > > > > supports userspace memory registration.
> > > > > > >
> > > > > > > Signed-off-by: Xie Yongji <xieyongji@...edance.com>
> > > > > > > ---
> > > > > > > drivers/vdpa/vdpa_user/vduse_dev.c | 39 ++++++++++++++++++++++++++++++
> > > > > > > include/uapi/linux/vduse.h | 24 ++++++++++++++++++
> > > > > > > 2 files changed, 63 insertions(+)
> > > > > > >
> > > > > > > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> > > > > > > index eedff0a3885a..e820c37dcba8 100644
> > > > > > > --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> > > > > > > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> > > > > > > @@ -1228,6 +1228,45 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
> > > > > > > umem.size);
> > > > > > > break;
> > > > > > > }
> > > > > > > + case VDUSE_IOTLB_GET_INFO: {
> > > > > > > + struct vduse_iova_info info;
> > > > > > > + struct vhost_iotlb_map *map;
> > > > > > > + struct vduse_iova_domain *domain = dev->domain;
> > > > > > > +
> > > > > > > + ret = -EFAULT;
> > > > > > > + if (copy_from_user(&info, argp, sizeof(info)))
> > > > > > > + break;
> > > > > > > +
> > > > > > > + ret = -EINVAL;
> > > > > > > + if (info.start > info.last)
> > > > > > > + break;
> > > > > > > +
> > > > > > > + if (!is_mem_zero((const char *)info.reserved,
> > > > > > > + sizeof(info.reserved)))
> > > > > > > + break;
> > > > > > > +
> > > > > > > + spin_lock(&domain->iotlb_lock);
> > > > > > > + map = vhost_iotlb_itree_first(domain->iotlb,
> > > > > > > + info.start, info.last);
> > > > > > > + if (map) {
> > > > > > > + info.start = map->start;
> > > > > > > + info.last = map->last;
> > > > > > > + info.capability = 0;
> > > > > > > + if (domain->bounce_map && map->start >= 0 &&
> > > > > > > + map->last < domain->bounce_size)
> > > > > > > + info.capability |= VDUSE_IOVA_CAP_UMEM;
> > > > > > > + }
> > > > > > > + spin_unlock(&domain->iotlb_lock);
> > > > > > > + if (!map)
> > > > > > > + break;
> > > > > > > +
> > > > > > > + ret = -EFAULT;
> > > > > > > + if (copy_to_user(argp, &info, sizeof(info)))
> > > > > > > + break;
> > > > > > > +
> > > > > > > + ret = 0;
> > > > > > > + break;
> > > > > > > + }
> > > > > > > default:
> > > > > > > ret = -ENOIOCTLCMD;
> > > > > > > break;
> > > > > > > diff --git a/include/uapi/linux/vduse.h b/include/uapi/linux/vduse.h
> > > > > > > index 9885e0571f09..11bd48c72c6c 100644
> > > > > > > --- a/include/uapi/linux/vduse.h
> > > > > > > +++ b/include/uapi/linux/vduse.h
> > > > > > > @@ -233,6 +233,30 @@ struct vduse_iova_umem {
> > > > > > > /* De-register the userspace memory. Caller should set iova and size field. */
> > > > > > > #define VDUSE_IOTLB_DEREG_UMEM _IOW(VDUSE_BASE, 0x19, struct vduse_iova_umem)
> > > > > > >
> > > > > > > +/**
> > > > > > > + * struct vduse_iova_info - information of one IOVA region
> > > > > > > + * @start: start of the IOVA region
> > > > > > > + * @last: last of the IOVA region
> > > > > > > + * @capability: capability of the IOVA regsion
> > > > > > > + * @reserved: for future use, needs to be initialized to zero
> > > > > > > + *
> > > > > > > + * Structure used by VDUSE_IOTLB_GET_INFO ioctl to get information of
> > > > > > > + * one IOVA region.
> > > > > > > + */
> > > > > > > +struct vduse_iova_info {
> > > > > > > + __u64 start;
> > > > > > > + __u64 last;
> > > > > > > +#define VDUSE_IOVA_CAP_UMEM (1 << 0)
> > > > > > > + __u64 capability;
> > > > > > > + __u64 reserved[3];
> > > > > > > +};
> > > > > > > +
> > > > > > > +/*
> > > > > > > + * Find the first IOVA region that overlaps with the range [start, last]
> > > > > >
> > > > > > So the code is actually find the IOVA region that is the super range
> > > > > > of [start, last] instead of overlap:
> > > > > >
> > > > >
> > > > > This is achieved by vhost_iotlb_itree_first(). And can't the super
> > > > > range of [start,last] be considered overlapping?
> > > >
> > > > Ok, but what I want to ask is, under which condition can we hit the
> > > > following case
> > > >
> > > > map->last >= domain->bounce_size ?
> > > >
> > >
> > > I think we would not hit this case currently.
> >
> > I wonder if it's worthwhile to have a WARN or just remove this check.
> >
>
> I think we can't remove the check since not only the bounce region
> will match the condition: map->start >= 0.
>
> Or changing to map->start == 0 && map->last == domain->bounce_size - 1 ?
This looks like an assumption that there should be only one single map
for bound buffer.
It should be fine.
Thanks
>
> Thanks,
> Yongji
>
Powered by blists - more mailing lists