linux-kernel - Re: [PATCH v3 01/10] vfio: Make vfio_unpin

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 12 Jul 2022 11:18:55 -0700
From:   Nicolin Chen <nicolinc@...dia.com>
To:     Anthony Krowiak <akrowiak@...ux.ibm.com>
CC:     <kwankhede@...dia.com>, <corbet@....net>, <hca@...ux.ibm.com>,
        <gor@...ux.ibm.com>, <agordeev@...ux.ibm.com>,
        <borntraeger@...ux.ibm.com>, <svens@...ux.ibm.com>,
        <zhenyuw@...ux.intel.com>, <zhi.a.wang@...el.com>,
        <jani.nikula@...ux.intel.com>, <joonas.lahtinen@...ux.intel.com>,
        <rodrigo.vivi@...el.com>, <tvrtko.ursulin@...ux.intel.com>,
        <airlied@...ux.ie>, <daniel@...ll.ch>, <farman@...ux.ibm.com>,
        <mjrosato@...ux.ibm.com>, <pasic@...ux.ibm.com>,
        <vneethv@...ux.ibm.com>, <oberpar@...ux.ibm.com>,
        <freude@...ux.ibm.com>, <jjherne@...ux.ibm.com>,
        <alex.williamson@...hat.com>, <cohuck@...hat.com>,
        <jgg@...dia.com>, <kevin.tian@...el.com>, <hch@...radead.org>,
        <jchrist@...ux.ibm.com>, <kvm@...r.kernel.org>,
        <linux-doc@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <linux-s390@...r.kernel.org>,
        <intel-gvt-dev@...ts.freedesktop.org>,
        <intel-gfx@...ts.freedesktop.org>,
        <dri-devel@...ts.freedesktop.org>, <terrence.xu@...el.com>
Subject: Re: [PATCH v3 01/10] vfio: Make vfio_unpin_pages() return void

On Tue, Jul 12, 2022 at 10:21:14AM -0400, Anthony Krowiak wrote:

> > +void vfio_unpin_pages(struct vfio_device *device, unsigned long *user_pfn,
> > +                   int npage)
> >   {
> >       struct vfio_container *container;
> >       struct vfio_iommu_driver *driver;
> > -     int ret;
> > 
> > -     if (!user_pfn || !npage || !vfio_assert_device_open(device))
> > -             return -EINVAL;
> 
> 
> You left out the check for !user_pfn?

Yes. I mentioned in the commit log. And it's in response to Jason's
remark: https://lore.kernel.org/kvm/20220707192210.GC1705032@nvidia.com/

Btw, user_pfn is removed in one of the following patches anyway.
 
> > +static void vfio_iommu_type1_unpin_pages(void *iommu_data,
> > +                                      unsigned long *user_pfn, int npage)
> >   {
> >       struct vfio_iommu *iommu = iommu_data;
> >       bool do_accounting;
> >       int i;
> > 
> > -     if (!iommu || !user_pfn || npage <= 0)
> > -             return -EINVAL;
> 
> 
> Is there a reason the checks above were not checked for WARN_ON?

For pointers, same reason here.

For npage, it's checked in its caller vfio_unpin_pages -- mentioned
in the commit log too. The VFIO core is the only caller and it is
unlikely to change. On the other hand, the plan is to replace this
vfio_iommu_type1_unpin_pages with IOMMUFD implementation.