Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH] crypto: testmgr - allocate buffers with __GFP_COMP
To:     Russell King - ARM Linux admin <linux@armlinux.org.uk>,
        Matthew Wilcox <willy@infradead.org>
Cc:     Kees Cook <keescook@chromium.org>, Rik van Riel <riel@surriel.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Eric Biggers <ebiggers@kernel.org>,
        Linux-MM <linux-mm@kvack.org>,
        linux-security-module <linux-security-module@vger.kernel.org>,
        Geert Uytterhoeven <geert@linux-m68k.org>,
        linux-crypto <linux-crypto@vger.kernel.org>,
        Dmitry Vyukov <dvyukov@google.com>,
        Laura Abbott <labbott@redhat.com>,
        Linux ARM <linux-arm-kernel@lists.infradead.org>,
        Herbert Xu <herbert@gondor.apana.org.au>
References: <20190411192607.GD225654@gmail.com>
 <20190411192827.72551-1-ebiggers@kernel.org>
 <CAGXu5jJ8k7fP5Vb=ygmQ0B45GfrK2PeaV04bPWmcZ6Vb+swgyA@mail.gmail.com>
 <20190415022412.GA29714@bombadil.infradead.org>
 <20190415024615.f765e7oagw26ezam@gondor.apana.org.au>
 <20190416021852.GA18616@bombadil.infradead.org>
 <CAGXu5jKaVB=bTJCBWhsxAny7-OkzXQ+8KCd5O+_-7hKcJFiqKw@mail.gmail.com>
 <20190417040822.GB7751@bombadil.infradead.org>
 <20190417080919.54wywpzrt3psn4vj@shell.armlinux.org.uk>
From:   Robin Murphy <robin.murphy@arm.com>
Message-ID: <0ea51c23-d285-25c3-80d6-f3c0045ee325@arm.com>
Date:   Wed, 17 Apr 2019 10:54:39 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.6.1
MIME-Version: 1.0
In-Reply-To: <20190417080919.54wywpzrt3psn4vj@shell.armlinux.org.uk>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-GB
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 17/04/2019 09:09, Russell King - ARM Linux admin wrote:
> On Tue, Apr 16, 2019 at 09:08:22PM -0700, Matthew Wilcox wrote:
>> On Mon, Apr 15, 2019 at 10:14:51PM -0500, Kees Cook wrote:
>>> On Mon, Apr 15, 2019 at 9:18 PM Matthew Wilcox <willy@infradead.org> wrote:
>>>> I agree; if the crypto code is never going to try to go from the address of
>>>> a byte in the allocation back to the head page, then there's no need to
>>>> specify GFP_COMP.
>>>>
>>>> But that leaves us in the awkward situation where
>>>> HARDENED_USERCOPY_PAGESPAN does need to be able to figure out whether
>>>> 'ptr + n - 1' lies within the same allocation as ptr.  Without using
>>>> a compound page, there's no indication in the VM structures that these
>>>> two pages were allocated as part of the same allocation.
>>>>
>>>> We could force all multi-page allocations to be compound pages if
>>>> HARDENED_USERCOPY_PAGESPAN is enabled, but I worry that could break
>>>> something.  We could make it catch fewer problems by succeeding if the
>>>> page is not compound.  I don't know, these all seem like bad choices
>>>> to me.
>>>
>>> If GFP_COMP is _not_ the correct signal about adjacent pages being
>>> part of the same allocation, then I agree: we need to drop this check
>>> entirely from PAGESPAN. Is there anything else that indicates this
>>> property? (Or where might we be able to store that info?)
>>
>> As far as I know, the page allocator does not store size information
>> anywhere, unless you use GFP_COMP.  That's why you have to pass
>> the 'order' to free_pages() and __free_pages().  It's also why
>> alloc_pages_exact() works (follow all the way into split_page()).
>>
>>> There are other pagespan checks, though, so those could stay. But I'd
>>> really love to gain page allocator allocation size checking ...
>>
>> I think that's a great idea, but I'm not sure how you'll be able to
>> do that.
> 
> However, we have had code (maybe historically now) that has allocated
> a higher order page and then handed back pages that it doesn't need -
> for example, when the code requires multiple contiguous pages but does
> not require a power-of-2 size of contiguous pages.

'git grep alloc_pages_exact' suggests it's not historical yet...

Robin.