MIME-Version: 1.0
References: <cover.1671098103.git.baskov@ispras.ru> <721b5c42e0e79d307d5bcb08f9c8402f5067ded0.1671098103.git.baskov@ispras.ru>
In-Reply-To: <721b5c42e0e79d307d5bcb08f9c8402f5067ded0.1671098103.git.baskov@ispras.ru>
From:   Ard Biesheuvel <ardb@kernel.org>
Date:   Fri, 10 Mar 2023 15:45:19 +0100
Message-ID: <CAMj1kXEdD8UT5SMU+3RH1qoNGcAOSF7fWnJS=Svy0EUB76FYng@mail.gmail.com>
Subject: Re: [PATCH v4 02/26] x86/build: Remove RWX sections and align on 4KB
To:     Evgeniy Baskov <baskov@ispras.ru>
Cc:     Borislav Petkov <bp@alien8.de>, Andy Lutomirski <luto@kernel.org>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        Ingo Molnar <mingo@redhat.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Alexey Khoroshilov <khoroshilov@ispras.ru>,
        Peter Jones <pjones@redhat.com>,
        "Limonciello, Mario" <mario.limonciello@amd.com>,
        joeyli <jlee@suse.com>, lvc-project@linuxtesting.org,
        x86@kernel.org, linux-efi@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, 15 Dec 2022 at 13:38, Evgeniy Baskov <baskov@ispras.ru> wrote:
>
> Avoid creating sections simultaneously writable and readable
> to prepare for W^X implementation. Align sections on page size (4KB) to
> allow protecting them in the page tables.
>
> Split init code form ".init" segment into separate R_X ".inittext"
> segment and make ".init" segment non-executable.
>
> Also add these segments to x86_32 architecture for consistency.
> Currently paging is disabled in x86_32 in compressed kernel, so
> protection is not applied anyways, but .init code was incorrectly
> placed in non-executable ".data" segment. This should not change
> anything meaningful in memory layout now, but might be required in case
> memory protection will also be implemented in compressed kernel for
> x86_32.
>
> Tested-by: Mario Limonciello <mario.limonciello@amd.com>
> Tested-by: Peter Jones <pjones@redhat.com>
> Signed-off-by: Evgeniy Baskov <baskov@ispras.ru>

Reviewed-by: Ard Biesheuvel <ardb@kernel.org>

One nit: the series modifies both the decompressor linker script and
the core kernel one, so please make it very explicit in the commit log
which one is being modified, and why it matters for this particular
context.


> ---
>  arch/x86/kernel/vmlinux.lds.S | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
> index 2e0ee14229bf..2e56d694c491 100644
> --- a/arch/x86/kernel/vmlinux.lds.S
> +++ b/arch/x86/kernel/vmlinux.lds.S
> @@ -102,12 +102,11 @@ jiffies = jiffies_64;
>  PHDRS {
>         text PT_LOAD FLAGS(5);          /* R_E */
>         data PT_LOAD FLAGS(6);          /* RW_ */
> -#ifdef CONFIG_X86_64
> -#ifdef CONFIG_SMP
> +#if defined(CONFIG_X86_64) && defined(CONFIG_SMP)
>         percpu PT_LOAD FLAGS(6);        /* RW_ */
>  #endif
> -       init PT_LOAD FLAGS(7);          /* RWE */
> -#endif
> +       inittext PT_LOAD FLAGS(5);      /* R_E */
> +       init PT_LOAD FLAGS(6);          /* RW_ */
>         note PT_NOTE FLAGS(0);          /* ___ */
>  }
>
> @@ -227,9 +226,10 @@ SECTIONS
>  #endif
>
>         INIT_TEXT_SECTION(PAGE_SIZE)
> -#ifdef CONFIG_X86_64
> -       :init
> -#endif
> +       :inittext
> +
> +       . = ALIGN(PAGE_SIZE);
> +
>
>         /*
>          * Section for code used exclusively before alternatives are run. All
> @@ -241,6 +241,7 @@ SECTIONS
>         .altinstr_aux : AT(ADDR(.altinstr_aux) - LOAD_OFFSET) {
>                 *(.altinstr_aux)
>         }
> +       :init
>
>         INIT_DATA_SECTION(16)
>
> --
> 2.37.4
>