linux-kernel - Re: [PATCH v2 05/10] iommufd: Add replace support in iommufd_access_set

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Thu, 9 Feb 2023 12:28:45 -0800
From:   Nicolin Chen <nicolinc@...dia.com>
To:     "jgg@...dia.com" <jgg@...dia.com>,
        "Tian, Kevin" <kevin.tian@...el.com>
CC:     "joro@...tes.org" <joro@...tes.org>,
        "will@...nel.org" <will@...nel.org>,
        "robin.murphy@....com" <robin.murphy@....com>,
        "alex.williamson@...hat.com" <alex.williamson@...hat.com>,
        "shuah@...nel.org" <shuah@...nel.org>,
        "Liu, Yi L" <yi.l.liu@...el.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "iommu@...ts.linux.dev" <iommu@...ts.linux.dev>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "linux-kselftest@...r.kernel.org" <linux-kselftest@...r.kernel.org>,
        "baolu.lu@...ux.intel.com" <baolu.lu@...ux.intel.com>
Subject: Re: [PATCH v2 05/10] iommufd: Add replace support in
 iommufd_access_set_ioas()

On Thu, Feb 09, 2023 at 03:13:08AM +0000, Tian, Kevin wrote:
 
> > --- a/drivers/iommu/iommufd/device.c
> > +++ b/drivers/iommu/iommufd/device.c
> > @@ -509,11 +509,23 @@ int iommufd_access_set_ioas(struct
> > iommufd_access *access, u32 ioas_id)
> >               iommufd_ref_to_users(obj);
> >       }
> >
> > +     /*
> > +      * Set ioas to NULL to block any further iommufd_access_pin_pages().
> > +      * iommufd_access_unpin_pages() can continue using access-
> > >ioas_unpin.
> > +      */
> > +     access->ioas = NULL;
> > +
> >       if (cur_ioas) {
> > +             if (new_ioas) {
> > +                     mutex_unlock(&access->ioas_lock);
> > +                     access->ops->unmap(access->data, 0, ULONG_MAX);
> > +                     mutex_lock(&access->ioas_lock);
> > +             }
> 
> why does above only apply to a valid new_ioas? this is the cleanup on
> cur_ioas then required even when new_ioas=NULL.
  
Though it'd make sense to put it in the common path, our current
detach routine doesn't call this unmap. If we do so, it'd become
something new to the normal detach routine. Or does this mean the
detach routine has been missing an unmap call so far?

Thanks
Nic