From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752035AbdLEUOt (ORCPT ); Tue, 5 Dec 2017 15:14:49 -0500 Received: from mail-ua0-f194.google.com ([209.85.217.194]:46689 "EHLO mail-ua0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751416AbdLEUOs (ORCPT ); Tue, 5 Dec 2017 15:14:48 -0500 X-Google-Smtp-Source: AGs4zMaBIGsjO6GAT3vl67eLYQeXG+UeVioMaGxbobZuw61r5nOjiLjLkOE6v3MCWLfZHPnrLkPx1N/jgNGN9hPDhn4= MIME-Version: 1.0 In-Reply-To: <20171205200935.GY10595@n2100.armlinux.org.uk> References: <20170823135836.52fb44fc@gandalf.local.home> <20170823150351.606ba09f@gandalf.local.home> <20171205114709.f6aj6i426keq2cn5@camel2.lan> <20171205131416.GW10595@n2100.armlinux.org.uk> <20171205132339.behn34z6b7ci2m4j@camel2.lan> <5b9b86cf-4b62-c984-fe52-a22df8fce33c@raspberrypi.org> <20171205133601.GX10595@n2100.armlinux.org.uk> <20171205200935.GY10595@n2100.armlinux.org.uk> From: Kees Cook Date: Tue, 5 Dec 2017 12:14:46 -0800 X-Google-Sender-Auth: nMrg7MrvSfY5HSITjoWcyMk8ues Message-ID: Subject: Re: [PATCH] Arm: mm: ftrace: Only set text back to ro after kernel has been marked ro To: Russell King - ARM Linux Cc: Stefan Wahren , Eric Anholt , Phil Elwell , LKML , Steven Rostedt , Matthias Reichl , linux-rpi-kernel@lists.infradead.org, "linux-arm-kernel@lists.infradead.org" Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Dec 5, 2017 at 12:09 PM, Russell King - ARM Linux wrote: > On Tue, Dec 05, 2017 at 11:35:59AM -0800, Kees Cook wrote: >> We don't _need_ to, but they're all contiguous, so the ro_perms array >> used by set_kernel_text_*() is actually only a single entry: >> >> static struct section_perm ro_perms[] = { >> /* Make kernel code and rodata RX (set RO). */ >> { >> .name = "text/rodata RO", >> .start = (unsigned long)_stext, >> .end = (unsigned long)__init_begin, >> ... > > Well, they may not be contiguous - it depends on DEBUG_ALIGN_RODATA. Maybe I'm picking a slightly wrong word. I guess I meant adjacent. The range _stext to __init_begin is all read-only, though there may be padding (controlled by DEBUG_ALIGN_RODATA), to allow a split for NX markings on rodata. > Either way, we have __start_rodata_section_aligned, which is either > the start of the read-only data section, or the start of the first > section beyond __start_rodata if DEBUG_ALIGN_RODATA is not set. > > Given that __start_rodata_section_aligned will always be less than > __init_begin, is there any reason not to make the above end at > __start_rodata_section_aligned, thereby allowing more of the read-only > data (in the case of DEBUG_ALIGN_RODATA=n) or all of the read-only > data (in the case of DEBUG_ALIGN_RODATA=y) to remain write-protected? Sure, there's no reason not to split this into two entries. It'll require some reworking of the function calls to get it right, obviously. -Kees -- Kees Cook Pixel Security From mboxrd@z Thu Jan 1 00:00:00 1970 From: keescook@chromium.org (Kees Cook) Date: Tue, 5 Dec 2017 12:14:46 -0800 Subject: [PATCH] Arm: mm: ftrace: Only set text back to ro after kernel has been marked ro In-Reply-To: <20171205200935.GY10595@n2100.armlinux.org.uk> References: <20170823135836.52fb44fc@gandalf.local.home> <20170823150351.606ba09f@gandalf.local.home> <20171205114709.f6aj6i426keq2cn5@camel2.lan> <20171205131416.GW10595@n2100.armlinux.org.uk> <20171205132339.behn34z6b7ci2m4j@camel2.lan> <5b9b86cf-4b62-c984-fe52-a22df8fce33c@raspberrypi.org> <20171205133601.GX10595@n2100.armlinux.org.uk> <20171205200935.GY10595@n2100.armlinux.org.uk> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Tue, Dec 5, 2017 at 12:09 PM, Russell King - ARM Linux wrote: > On Tue, Dec 05, 2017 at 11:35:59AM -0800, Kees Cook wrote: >> We don't _need_ to, but they're all contiguous, so the ro_perms array >> used by set_kernel_text_*() is actually only a single entry: >> >> static struct section_perm ro_perms[] = { >> /* Make kernel code and rodata RX (set RO). */ >> { >> .name = "text/rodata RO", >> .start = (unsigned long)_stext, >> .end = (unsigned long)__init_begin, >> ... > > Well, they may not be contiguous - it depends on DEBUG_ALIGN_RODATA. Maybe I'm picking a slightly wrong word. I guess I meant adjacent. The range _stext to __init_begin is all read-only, though there may be padding (controlled by DEBUG_ALIGN_RODATA), to allow a split for NX markings on rodata. > Either way, we have __start_rodata_section_aligned, which is either > the start of the read-only data section, or the start of the first > section beyond __start_rodata if DEBUG_ALIGN_RODATA is not set. > > Given that __start_rodata_section_aligned will always be less than > __init_begin, is there any reason not to make the above end at > __start_rodata_section_aligned, thereby allowing more of the read-only > data (in the case of DEBUG_ALIGN_RODATA=n) or all of the read-only > data (in the case of DEBUG_ALIGN_RODATA=y) to remain write-protected? Sure, there's no reason not to split this into two entries. It'll require some reworking of the function calls to get it right, obviously. -Kees -- Kees Cook Pixel Security