From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751875AbcGOMzE (ORCPT ); Fri, 15 Jul 2016 08:55:04 -0400 Received: from mail-pf0-f195.google.com ([209.85.192.195]:35484 "EHLO mail-pf0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751032AbcGOMy7 (ORCPT ); Fri, 15 Jul 2016 08:54:59 -0400 Date: Fri, 15 Jul 2016 22:55:19 +1000 From: Balbir Singh To: Kees Cook Cc: Balbir Singh , Rik van Riel , LKML , Casey Schaufler , PaX Team , Brad Spengler , Russell King , Catalin Marinas , Will Deacon , Ard Biesheuvel , Benjamin Herrenschmidt , Michael Ellerman , Tony Luck , Fenghua Yu , "David S. Miller" , "x86@kernel.org" , Christoph Lameter , Pekka Enberg , David Rientjes , Joonsoo Kim , Andrew Morton , Andy Lutomirski , Borislav Petkov , Mathias Krause , Jan Kara , Vitaly Wool , Andrea Arcangeli , Dmitry Vyukov , Laura Abbott , "linux-arm-kernel@lists.infradead.org" , linux-ia64@vger.kernel.org, "linuxppc-dev@lists.ozlabs.org" , sparclinux , linux-arch , Linux-MM , "kernel-hardening@lists.openwall.com" Subject: Re: [PATCH v2 02/11] mm: Hardened usercopy Message-ID: <20160715125519.GA21685@350D> Reply-To: bsingharora@gmail.com References: <1468446964-22213-1-git-send-email-keescook@chromium.org> <1468446964-22213-3-git-send-email-keescook@chromium.org> <20160714232019.GA28254@350D> <1468544658.30053.26.camel@redhat.com> <20160715014151.GA13944@balbir.ozlabs.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Jul 14, 2016 at 09:53:31PM -0700, Kees Cook wrote: > On Thu, Jul 14, 2016 at 9:05 PM, Kees Cook wrote: > > On Thu, Jul 14, 2016 at 6:41 PM, Balbir Singh 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 ""; > > } > > > > 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 ""; > } > > return NULL; That looks reasonable to me Balbir