Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAGXu5jJy4O4tV2sB=MSkYh2DEfxUYkH4Q3ghmrHEGc6s-k285A@mail.gmail.com>
Date: Thu, 14 Jul 2016 21:05:25 -0700
From: Kees Cook <keescook@...omium.org>
To: bsingharora@...il.com
Cc: Rik van Riel <riel@...hat.com>, LKML <linux-kernel@...r.kernel.org>, 
	Casey Schaufler <casey@...aufler-ca.com>, PaX Team <pageexec@...email.hu>, 
	Brad Spengler <spender@...ecurity.net>, Russell King <linux@...linux.org.uk>, 
	Catalin Marinas <catalin.marinas@....com>, Will Deacon <will.deacon@....com>, 
	Ard Biesheuvel <ard.biesheuvel@...aro.org>, 
	Benjamin Herrenschmidt <benh@...nel.crashing.org>, Michael Ellerman <mpe@...erman.id.au>, 
	Tony Luck <tony.luck@...el.com>, Fenghua Yu <fenghua.yu@...el.com>, 
	"David S. Miller" <davem@...emloft.net>, "x86@...nel.org" <x86@...nel.org>, Christoph Lameter <cl@...ux.com>, 
	Pekka Enberg <penberg@...nel.org>, David Rientjes <rientjes@...gle.com>, 
	Joonsoo Kim <iamjoonsoo.kim@....com>, Andrew Morton <akpm@...ux-foundation.org>, 
	Andy Lutomirski <luto@...nel.org>, Borislav Petkov <bp@...e.de>, Mathias Krause <minipli@...glemail.com>, 
	Jan Kara <jack@...e.cz>, Vitaly Wool <vitalywool@...il.com>, 
	Andrea Arcangeli <aarcange@...hat.com>, Dmitry Vyukov <dvyukov@...gle.com>, 
	Laura Abbott <labbott@...oraproject.org>, 
	"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>, linux-ia64@...r.kernel.org, 
	"linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>, sparclinux <sparclinux@...r.kernel.org>, 
	linux-arch <linux-arch@...r.kernel.org>, Linux-MM <linux-mm@...ck.org>, 
	"kernel-hardening@...ts.openwall.com" <kernel-hardening@...ts.openwall.com>
Subject: Re: [PATCH v2 02/11] mm: Hardened usercopy

On Thu, Jul 14, 2016 at 6:41 PM, Balbir Singh <bsingharora@...il.com> wrote:
> On Thu, Jul 14, 2016 at 09:04:18PM -0400, Rik van Riel wrote:
>> On Fri, 2016-07-15 at 09:20 +1000, Balbir Singh wrote:
>>
>> > > ==
>> > > +            ((unsigned long)end & (unsigned
>> > > long)PAGE_MASK)))
>> > > +         return NULL;
>> > > +
>> > > + /* Allow if start and end are inside the same compound
>> > > page. */
>> > > + endpage = virt_to_head_page(end);
>> > > + if (likely(endpage == page))
>> > > +         return NULL;
>> > > +
>> > > + /* Allow special areas, device memory, and sometimes
>> > > kernel data. */
>> > > + if (PageReserved(page) && PageReserved(endpage))
>> > > +         return NULL;
>> >
>> > If we came here, it's likely that endpage > page, do we need to check
>> > that only the first and last pages are reserved? What about the ones
>> > in
>> > the middle?
>>
>> I think this will be so rare, we can get away with just
>> checking the beginning and the end.
>>
>
> But do we want to leave a hole where an aware user space
> can try a longer copy_* to avoid this check? If it is unlikely
> should we just bite the bullet and do the check for the entire
> range?

I'd be okay with expanding the test -- it should be an extremely rare
situation already since the common Reserved areas (kernel data) will
have already been explicitly tested.

What's the best way to do "next page"? Should it just be:

for ( ; page <= endpage ; ptr += PAGE_SIZE, page = virt_to_head_page(ptr) ) {
    if (!PageReserved(page))
        return "<spans multiple pages>";
}

return NULL;

?


-- 
Kees Cook
Chrome OS & Brillo Security

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.