From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752372AbcAGO0R (ORCPT ); Thu, 7 Jan 2016 09:26:17 -0500 Received: from foss.arm.com ([217.140.101.70]:40064 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751896AbcAGO0P (ORCPT ); Thu, 7 Jan 2016 09:26:15 -0500 Date: Thu, 7 Jan 2016 14:25:54 +0000 From: Mark Rutland To: Ard Biesheuvel Cc: "linux-arm-kernel@lists.infradead.org" , kernel-hardening@lists.openwall.com, Will Deacon , Catalin Marinas , Leif Lindholm , Kees Cook , "linux-kernel@vger.kernel.org" , Stuart Yoder , Sharma Bhupesh , Arnd Bergmann , Marc Zyngier , Christoffer Dall Subject: Re: [PATCH v2 03/13] arm64: use more granular reservations for static page table allocations Message-ID: <20160107142541.GC15917@leverpostej> References: <1451489172-17420-1-git-send-email-ard.biesheuvel@linaro.org> <1451489172-17420-4-git-send-email-ard.biesheuvel@linaro.org> <20160107135518.GB15917@leverpostej> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Jan 07, 2016 at 03:02:00PM +0100, Ard Biesheuvel wrote: > On 7 January 2016 at 14:55, Mark Rutland wrote: > > On Wed, Dec 30, 2015 at 04:26:02PM +0100, Ard Biesheuvel wrote: > >> Before introducing new statically allocated page tables and increasing > >> their alignment in subsequent patches, update the reservation logic > >> so that only pages that are in actual use end up as reserved with > >> memblock. > > > > Could you add something to the commit message about what this will gain > > us (i.e. which pages we don't have to reserve)? It's not immediately > > obvious why we'd have page tables we wouldn't want to reserve. > > > > OK. In the original series, I also aligned the pgdir section to a log2 > upper bound of its size, but that is not necessary anymore with your > changes. So the original goal was to avoid reserving the alignment > padding as well as the pgdirs that end up unused Ah, I see. > > From the looks of the next patch we won't have redundant levels of > > fixmap table for a given configuration, so I guess we're catering for > > the case the fixmap shares a pgd/pud/pmd entry with the image mapping? > > > > Does that happen? If so that would invalidate the assumption I make when > > copying the fixmap over in [1] (see map_kernel). > > > > It is a lot less likely to happen now that I moved the kernel to the > start of the vmalloc area rather than right below PAGE_OFFSET. But in > general, it seems sensible to only populate entries after confirming > that they are in fact vacant. Sure. > > To handle that either we need some special logic to copy over the > > relevant bits for the fixmap (as with kasan_copy_shadow), or we need to > > avoid sharing a pgd entry. > > > > Thoughts? > > > > Yes, I have added that to my v3 version of the vmalloc base move patch here > > https://git.linaro.org/people/ard.biesheuvel/linux-arm.git/commitdiff/0beef2c1a6bfc90cc116a6ba1b24f2ba35e7e5f6 Ah, great! > but I think 16k/4 levels is the only config affected when the kernel > is always in the lower half of the vmalloc area. That also implies > that the fixmap pgd is either always shared, or never, depending on > the build time config, so I could probably simplify that part > somewhat. Ok. Thanks, Mark. From mboxrd@z Thu Jan 1 00:00:00 1970 From: mark.rutland@arm.com (Mark Rutland) Date: Thu, 7 Jan 2016 14:25:54 +0000 Subject: [PATCH v2 03/13] arm64: use more granular reservations for static page table allocations In-Reply-To: References: <1451489172-17420-1-git-send-email-ard.biesheuvel@linaro.org> <1451489172-17420-4-git-send-email-ard.biesheuvel@linaro.org> <20160107135518.GB15917@leverpostej> Message-ID: <20160107142541.GC15917@leverpostej> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Thu, Jan 07, 2016 at 03:02:00PM +0100, Ard Biesheuvel wrote: > On 7 January 2016 at 14:55, Mark Rutland wrote: > > On Wed, Dec 30, 2015 at 04:26:02PM +0100, Ard Biesheuvel wrote: > >> Before introducing new statically allocated page tables and increasing > >> their alignment in subsequent patches, update the reservation logic > >> so that only pages that are in actual use end up as reserved with > >> memblock. > > > > Could you add something to the commit message about what this will gain > > us (i.e. which pages we don't have to reserve)? It's not immediately > > obvious why we'd have page tables we wouldn't want to reserve. > > > > OK. In the original series, I also aligned the pgdir section to a log2 > upper bound of its size, but that is not necessary anymore with your > changes. So the original goal was to avoid reserving the alignment > padding as well as the pgdirs that end up unused Ah, I see. > > From the looks of the next patch we won't have redundant levels of > > fixmap table for a given configuration, so I guess we're catering for > > the case the fixmap shares a pgd/pud/pmd entry with the image mapping? > > > > Does that happen? If so that would invalidate the assumption I make when > > copying the fixmap over in [1] (see map_kernel). > > > > It is a lot less likely to happen now that I moved the kernel to the > start of the vmalloc area rather than right below PAGE_OFFSET. But in > general, it seems sensible to only populate entries after confirming > that they are in fact vacant. Sure. > > To handle that either we need some special logic to copy over the > > relevant bits for the fixmap (as with kasan_copy_shadow), or we need to > > avoid sharing a pgd entry. > > > > Thoughts? > > > > Yes, I have added that to my v3 version of the vmalloc base move patch here > > https://git.linaro.org/people/ard.biesheuvel/linux-arm.git/commitdiff/0beef2c1a6bfc90cc116a6ba1b24f2ba35e7e5f6 Ah, great! > but I think 16k/4 levels is the only config affected when the kernel > is always in the lower half of the vmalloc area. That also implies > that the fixmap pgd is either always shared, or never, depending on > the build time config, so I could probably simplify that part > somewhat. Ok. Thanks, Mark. From mboxrd@z Thu Jan 1 00:00:00 1970 Reply-To: kernel-hardening@lists.openwall.com Date: Thu, 7 Jan 2016 14:25:54 +0000 From: Mark Rutland Message-ID: <20160107142541.GC15917@leverpostej> References: <1451489172-17420-1-git-send-email-ard.biesheuvel@linaro.org> <1451489172-17420-4-git-send-email-ard.biesheuvel@linaro.org> <20160107135518.GB15917@leverpostej> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: [kernel-hardening] Re: [PATCH v2 03/13] arm64: use more granular reservations for static page table allocations To: Ard Biesheuvel Cc: "linux-arm-kernel@lists.infradead.org" , kernel-hardening@lists.openwall.com, Will Deacon , Catalin Marinas , Leif Lindholm , Kees Cook , "linux-kernel@vger.kernel.org" , Stuart Yoder , Sharma Bhupesh , Arnd Bergmann , Marc Zyngier , Christoffer Dall List-ID: On Thu, Jan 07, 2016 at 03:02:00PM +0100, Ard Biesheuvel wrote: > On 7 January 2016 at 14:55, Mark Rutland wrote: > > On Wed, Dec 30, 2015 at 04:26:02PM +0100, Ard Biesheuvel wrote: > >> Before introducing new statically allocated page tables and increasing > >> their alignment in subsequent patches, update the reservation logic > >> so that only pages that are in actual use end up as reserved with > >> memblock. > > > > Could you add something to the commit message about what this will gain > > us (i.e. which pages we don't have to reserve)? It's not immediately > > obvious why we'd have page tables we wouldn't want to reserve. > > > > OK. In the original series, I also aligned the pgdir section to a log2 > upper bound of its size, but that is not necessary anymore with your > changes. So the original goal was to avoid reserving the alignment > padding as well as the pgdirs that end up unused Ah, I see. > > From the looks of the next patch we won't have redundant levels of > > fixmap table for a given configuration, so I guess we're catering for > > the case the fixmap shares a pgd/pud/pmd entry with the image mapping? > > > > Does that happen? If so that would invalidate the assumption I make when > > copying the fixmap over in [1] (see map_kernel). > > > > It is a lot less likely to happen now that I moved the kernel to the > start of the vmalloc area rather than right below PAGE_OFFSET. But in > general, it seems sensible to only populate entries after confirming > that they are in fact vacant. Sure. > > To handle that either we need some special logic to copy over the > > relevant bits for the fixmap (as with kasan_copy_shadow), or we need to > > avoid sharing a pgd entry. > > > > Thoughts? > > > > Yes, I have added that to my v3 version of the vmalloc base move patch here > > https://git.linaro.org/people/ard.biesheuvel/linux-arm.git/commitdiff/0beef2c1a6bfc90cc116a6ba1b24f2ba35e7e5f6 Ah, great! > but I think 16k/4 levels is the only config affected when the kernel > is always in the lower half of the vmalloc area. That also implies > that the fixmap pgd is either always shared, or never, depending on > the build time config, so I could probably simplify that part > somewhat. Ok. Thanks, Mark.