|
Message-ID: <20160715125519.GA21685@350D> Date: Fri, 15 Jul 2016 22:55:19 +1000 From: Balbir Singh <bsingharora@...il.com> To: Kees Cook <keescook@...omium.org> Cc: Balbir Singh <bsingharora@...il.com>, 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 09:53:31PM -0700, Kees Cook wrote: > On Thu, Jul 14, 2016 at 9:05 PM, Kees Cook <keescook@...omium.org> wrote: > > 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; > > > > ? > > Er, I was testing the wrong thing. How about: > > /* > * Reject if range is not Reserved (i.e. special or device memory), > * since then the object spans several independently allocated pages. > */ > for (; ptr <= end ; ptr += PAGE_SIZE, page = virt_to_head_page(ptr)) { > if (!PageReserved(page)) > return "<spans multiple pages>"; > } > > return NULL; That looks reasonable to me Balbir
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.