linux-kernel - Re: [PATCH 1/2] mm: reorganize internal_get_user_pages

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] [day] [month] [year] [list]

Date:   Tue, 27 Oct 2020 23:00:40 -0700
From:   John Hubbard <jhubbard@...dia.com>
To:     Christoph Hellwig <hch@....de>, Jan Kara <jack@...e.cz>
CC:     Jason Gunthorpe <jgg@...dia.com>, <linux-kernel@...r.kernel.org>,
        "Andrea Arcangeli" <aarcange@...hat.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Hugh Dickins <hughd@...gle.com>, Jann Horn <jannh@...gle.com>,
        "Kirill Shutemov" <kirill@...temov.name>,
        Kirill Tkhai <ktkhai@...tuozzo.com>,
        Linux-MM <linux-mm@...ck.org>, Michal Hocko <mhocko@...e.com>,
        Oleg Nesterov <oleg@...hat.com>, Peter Xu <peterx@...hat.com>
Subject: Re: [PATCH 1/2] mm: reorganize internal_get_user_pages_fast()

On 10/27/20 2:55 AM, Christoph Hellwig wrote:
> On Tue, Oct 27, 2020 at 10:33:01AM +0100, Jan Kara wrote:
>> Actually there are callers that care about partial success. See e.g.
>> iov_iter_get_pages() usage in fs/direct_io.c:dio_refill_pages() or
>> bio_iov_iter_get_pages(). These places handle partial success just fine and
>> not allowing partial success from GUP could regress things...

Good point. And those also happen to be the key call sites that I haven't
yet converted to pin_user_pages*(). Seeing as how I'm three versions into
attempting to convert the various *iov_iter*() routines, I should have
remembered that they are all about partial success. :)

> 
> But most users do indeed not care.  Maybe an explicit FOLL_PARTIAL to
> opt into partial handling could clean up a lot of the mess.  Maybe just
> for pin_user_pages for now.
> 

That does seem like the perfect mix. IIRC, all of the pin_user_pages()
call sites  today do not accept partial success (and it's easy enough to
audit and confirm). So likely no need to add FOLL_PARTIAL there, and no
huge danger of regressions. It would definitely reduce the line count at
multiple call sites, in return for adding some lines to gup.c.

And maybe it can go further, at some point, but that's a good way to start.

I'm leaning toward just sending out a small series to do that, unless there
are objections and/or better ways to improve this area...

thanks,
-- 
John Hubbard
NVIDIA