linux-kernel - Re: [PATCH] vfio/pci: make the vfio_pci_mmap

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]

Message-ID: <20210309082951.75f0eb01@x1.home.shazbot.org>
Date:   Tue, 9 Mar 2021 08:29:51 -0700
From:   Alex Williamson <alex.williamson@...hat.com>
To:     Jason Gunthorpe <jgg@...dia.com>
Cc:     "Zengtao (B)" <prime.zeng@...ilicon.com>,
        Peter Xu <peterx@...hat.com>,
        Cornelia Huck <cohuck@...hat.com>,
        Kevin Tian <kevin.tian@...el.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Giovanni Cabiddu <giovanni.cabiddu@...el.com>,
        Michel Lespinasse <walken@...gle.com>,
        Jann Horn <jannh@...gle.com>,
        Max Gurtovoy <mgurtovoy@...dia.com>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Linuxarm <linuxarm@...wei.com>
Subject: Re: [PATCH] vfio/pci: make the vfio_pci_mmap_fault reentrant

On Tue, 9 Mar 2021 08:46:09 -0400
Jason Gunthorpe <jgg@...dia.com> wrote:

> On Tue, Mar 09, 2021 at 03:49:09AM +0000, Zengtao (B) wrote:
> > Hi guys:
> > 
> > Thanks for the helpful comments, after rethinking the issue, I have proposed
> >  the following change: 
> > 1. follow_pte instead of follow_pfn.  
> 
> Still no on follow_pfn, you don't need it once you use vmf_insert_pfn

vmf_insert_pfn() only solves the BUG_ON, follow_pte() is being used
here to determine whether the translation is already present to avoid
both duplicate work in inserting the translation and allocating a
duplicate vma tracking structure.

> > 2. vmf_insert_pfn loops instead of io_remap_pfn_range
> > 3. proper undos when some call fails.
> > 4. keep the bigger lock range to avoid unessary pte install.   
> 
> Why do we need locks at all here?

For the vma tracking and testing whether the fault is already
populated.  Once we get rid of the vma list, maybe it makes sense to
only insert the faulting page rather than the entire vma, at which
point I think we'd have no reason to serialize.  Thanks,

Alex