Re: [PATCH v4 22/26] arm64: mm: move ro_after_init section into the data segment

From: Ard Biesheuvel <ardb@kernel.org>
To: Kees Cook <keescook@chromium.org>
Cc: Linux ARM <linux-arm-kernel@lists.infradead.org>,
	linux-hardening@vger.kernel.org, Marc Zyngier <maz@kernel.org>,
	Will Deacon <will@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Mark Brown <broonie@kernel.org>,
	Anshuman Khandual <anshuman.khandual@arm.com>
Subject: Re: [PATCH v4 22/26] arm64: mm: move ro_after_init section into the data segment
Date: Thu, 16 Jun 2022 13:31:23 +0200	[thread overview]
Message-ID: <CAMj1kXHP8+yErY8F3wzc8=sWfqUYxCJ_AtW0_Gugng-0cpEXPA@mail.gmail.com> (raw)
In-Reply-To: <202206131630.B6AE6ECEA3@keescook>

On Tue, 14 Jun 2022 at 01:38, Kees Cook <keescook@chromium.org> wrote:
>
> On Mon, Jun 13, 2022 at 07:16:15PM +0200, Ard Biesheuvel wrote:
> > On Mon, 13 Jun 2022 at 19:00, Kees Cook <keescook@chromium.org> wrote:
> > >
> > > On Mon, Jun 13, 2022 at 04:45:46PM +0200, Ard Biesheuvel wrote:
> > > > Currently, the ro_after_init sections sits right in the middle of the
> > > > text/rodata/inittext segment, making it difficult to map any of those
> > > > non-writable during early boot. So instead, move it to the start of
> > > > .data, and update the init sequences so that the section is remapped
> > > > read-only once startup completes.
> > > >
> > > > Note that this moves the entire HYP data section into .data as well -
> > > > this likely needs to remain as a single block for now, but could perhaps
> > > > split into a .rodata and .data..ro_after_init section later.
> > >
> > > If I'm reading this correctly, this means that .data..ro_after_init now
> > > lives between .data and .rodata?
> > >
> >
> > No, between .initdata and .data
>
> Ah, doesn't this mean more padding (for segment alignment) used? On other
> architectures .data..ro_after_init tried to be near the writable/read-only
> boundary so segment padding was only needed on one side (e.g. it could
> live at the end of .rodata without segment alignment but before .data
> which was segment aligned.) Then when .rodata was made read-only (after
> __init), .data..ro_after_init would also get set read-only.
>
> In this case, I think it ends up needing segment alignment both at the
> front and the end, since the .initdata and .data are freed and left
> writable, respectively?
>

We used to have

text
--
rodata
(ro_after_init)
--
inittext
--
initdata
--
data
bss

where -- are the segment boundaries, which are always aligned to 64k on arm64

After this patch, we get

text
--
rodata
--
inittext
--
initdata
--
(ro_after_init)
data
bss

so in terms of padding due to alignment, there is not a lot of difference.

The main difference here is the fact that we lose the ability to use
block mappings, but if anyone cares about that, we could work around
this by creating a separate segment for ro_after_init.