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]
Message-ID: <d5140833-e9ee-beb5-ff0a-2d13a4fe819f@nvidia.com>
Date:   Thu, 1 Aug 2019 22:48:15 -0700
From:   John Hubbard <jhubbard@...dia.com>
To:     Juergen Gross <jgross@...e.com>, <john.hubbard@...il.com>,
        Andrew Morton <akpm@...ux-foundation.org>
CC:     <devel@...verdev.osuosl.org>, Dave Chinner <david@...morbit.com>,
        Christoph Hellwig <hch@...radead.org>,
        Dan Williams <dan.j.williams@...el.com>,
        Ira Weiny <ira.weiny@...el.com>, <x86@...nel.org>,
        <linux-mm@...ck.org>, Dave Hansen <dave.hansen@...ux.intel.com>,
        <amd-gfx@...ts.freedesktop.org>, <dri-devel@...ts.freedesktop.org>,
        <intel-gfx@...ts.freedesktop.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-rpi-kernel@...ts.infradead.org>, <devel@...ts.orangefs.org>,
        <xen-devel@...ts.xenproject.org>,
        Boris Ostrovsky <boris.ostrovsky@...cle.com>,
        <rds-devel@....oracle.com>,
        Jérôme Glisse <jglisse@...hat.com>,
        Jan Kara <jack@...e.cz>, <ceph-devel@...r.kernel.org>,
        <kvm@...r.kernel.org>, <linux-block@...r.kernel.org>,
        <linux-crypto@...r.kernel.org>, <linux-fbdev@...r.kernel.org>,
        <linux-fsdevel@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        <linux-media@...r.kernel.org>, <linux-nfs@...r.kernel.org>,
        <linux-rdma@...r.kernel.org>, <linux-xfs@...r.kernel.org>,
        <netdev@...r.kernel.org>, <sparclinux@...r.kernel.org>,
        Jason Gunthorpe <jgg@...pe.ca>
Subject: Re: [PATCH 20/34] xen: convert put_page() to put_user_page*()

On 8/1/19 9:36 PM, Juergen Gross wrote:
> On 02.08.19 04:19, john.hubbard@...il.com wrote:
>> From: John Hubbard <jhubbard@...dia.com>
...
>> diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
>> index 2f5ce7230a43..29e461dbee2d 100644
>> --- a/drivers/xen/privcmd.c
>> +++ b/drivers/xen/privcmd.c
>> @@ -611,15 +611,10 @@ static int lock_pages(
>>   static void unlock_pages(struct page *pages[], unsigned int nr_pages)
>>   {
>> -    unsigned int i;
>> -
>>       if (!pages)
>>           return;
>> -    for (i = 0; i < nr_pages; i++) {
>> -        if (pages[i])
>> -            put_page(pages[i]);
>> -    }
>> +    put_user_pages(pages, nr_pages);
> 
> You are not handling the case where pages[i] is NULL here. Or am I
> missing a pending patch to put_user_pages() here?
> 

Hi Juergen,

You are correct--this no longer handles the cases where pages[i]
is NULL. It's intentional, though possibly wrong. :)

I see that I should have added my standard blurb to this
commit description. I missed this one, but some of the other patches
have it. It makes the following, possibly incorrect claim:

"This changes the release code slightly, because each page slot in the
page_list[] array is no longer checked for NULL. However, that check
was wrong anyway, because the get_user_pages() pattern of usage here
never allowed for NULL entries within a range of pinned pages."

The way I've seen these page arrays used with get_user_pages(),
things are either done single page, or with a contiguous range. So
unless I'm missing a case where someone is either

a) releasing individual pages within a range (and thus likely messing
up their count of pages they have), or

b) allocating two gup ranges within the same pages[] array, with a
gap between the allocations,

...then it should be correct. If so, then I'll add the above blurb
to this patch's commit description.

If that's not the case (both here, and in 3 or 4 other patches in this
series, then as you said, I should add NULL checks to put_user_pages()
and put_user_pages_dirty_lock().


thanks,
-- 
John Hubbard
NVIDIA

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ