netdev - Re: [PATCH v7 09/24] vfio, mm: fix get_user_pages_remote() and FOLL

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, 21 Nov 2019 13:49:40 -0800
From:   John Hubbard <jhubbard@...dia.com>
To:     Alex Williamson <alex.williamson@...hat.com>
CC:     Andrew Morton <akpm@...ux-foundation.org>,
        Al Viro <viro@...iv.linux.org.uk>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Björn Töpel <bjorn.topel@...el.com>,
        Christoph Hellwig <hch@...radead.org>,
        Dan Williams <dan.j.williams@...el.com>,
        Daniel Vetter <daniel@...ll.ch>,
        Dave Chinner <david@...morbit.com>,
        David Airlie <airlied@...ux.ie>,
        "David S . Miller" <davem@...emloft.net>,
        Ira Weiny <ira.weiny@...el.com>, Jan Kara <jack@...e.cz>,
        Jason Gunthorpe <jgg@...pe.ca>, Jens Axboe <axboe@...nel.dk>,
        Jonathan Corbet <corbet@....net>,
        Jérôme Glisse <jglisse@...hat.com>,
        Magnus Karlsson <magnus.karlsson@...el.com>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Michael Ellerman <mpe@...erman.id.au>,
        Michal Hocko <mhocko@...e.com>,
        Mike Kravetz <mike.kravetz@...cle.com>,
        Paul Mackerras <paulus@...ba.org>,
        Shuah Khan <shuah@...nel.org>,
        Vlastimil Babka <vbabka@...e.cz>, <bpf@...r.kernel.org>,
        <dri-devel@...ts.freedesktop.org>, <kvm@...r.kernel.org>,
        <linux-block@...r.kernel.org>, <linux-doc@...r.kernel.org>,
        <linux-fsdevel@...r.kernel.org>, <linux-kselftest@...r.kernel.org>,
        <linux-media@...r.kernel.org>, <linux-rdma@...r.kernel.org>,
        <linuxppc-dev@...ts.ozlabs.org>, <netdev@...r.kernel.org>,
        <linux-mm@...ck.org>, LKML <linux-kernel@...r.kernel.org>,
        Jason Gunthorpe <jgg@...lanox.com>
Subject: Re: [PATCH v7 09/24] vfio, mm: fix get_user_pages_remote() and
 FOLL_LONGTERM

On 11/21/19 1:35 PM, Alex Williamson wrote:
> On Wed, 20 Nov 2019 23:13:39 -0800
> John Hubbard <jhubbard@...dia.com> wrote:
> 
>> As it says in the updated comment in gup.c: current FOLL_LONGTERM
>> behavior is incompatible with FAULT_FLAG_ALLOW_RETRY because of the
>> FS DAX check requirement on vmas.
>>
>> However, the corresponding restriction in get_user_pages_remote() was
>> slightly stricter than is actually required: it forbade all
>> FOLL_LONGTERM callers, but we can actually allow FOLL_LONGTERM callers
>> that do not set the "locked" arg.
>>
>> Update the code and comments accordingly, and update the VFIO caller
>> to take advantage of this, fixing a bug as a result: the VFIO caller
>> is logically a FOLL_LONGTERM user.
>>
>> Also, remove an unnessary pair of calls that were releasing and
>> reacquiring the mmap_sem. There is no need to avoid holding mmap_sem
>> just in order to call page_to_pfn().
>>
>> Also, move the DAX check ("if a VMA is DAX, don't allow long term
>> pinning") from the VFIO call site, all the way into the internals
>> of get_user_pages_remote() and __gup_longterm_locked(). That is:
>> get_user_pages_remote() calls __gup_longterm_locked(), which in turn
>> calls check_dax_vmas(). It's lightly explained in the comments as well.
>>
>> Thanks to Jason Gunthorpe for pointing out a clean way to fix this,
>> and to Dan Williams for helping clarify the DAX refactoring.
>>
>> Reviewed-by: Jason Gunthorpe <jgg@...lanox.com>
>> Reviewed-by: Ira Weiny <ira.weiny@...el.com>
>> Suggested-by: Jason Gunthorpe <jgg@...pe.ca>
>> Cc: Dan Williams <dan.j.williams@...el.com>
>> Cc: Jerome Glisse <jglisse@...hat.com>
>> Signed-off-by: John Hubbard <jhubbard@...dia.com>
>> ---
>>   drivers/vfio/vfio_iommu_type1.c | 30 +++++-------------------------
>>   mm/gup.c                        | 27 ++++++++++++++++++++++-----
>>   2 files changed, 27 insertions(+), 30 deletions(-)
> 
> Tested with device assignment and Intel mdev vGPU assignment with QEMU
> userspace:
> 
> Tested-by: Alex Williamson <alex.williamson@...hat.com>
> Acked-by: Alex Williamson <alex.williamson@...hat.com>
> 
> Feel free to include for 19/24 as well.  Thanks,
> 
> Alex


Great! Thanks for the testing and ack on those. I'm about to repackage
(and split up as CH requested) for 5.5, and will keep you on CC, of course.

thanks,
-- 
John Hubbard
NVIDIA