Subject: Re: [PATCH 2/7] powerpc: move __end_rodata to cover arch read-only sections

From: Michael Ellerman <[email-masked]>
Date: Fri, 16 Sep 2022 16:35:14 +1000

"Nicholas Piggin" <[email-masked]> writes:
> On Thu Sep 15, 2022 at 10:47 PM AEST, Michael Ellerman wrote:
>> Nicholas Piggin <[email-masked]> writes:
>> > powerpc has a number of read-only sections and tables that are put
>> > after RO_DATA(). Move the __end_rodata symbol to cover these as well.
>> >
>> > Setting memory to read-only at boot is done using __init_begin,
>> > change that that to use __end_rodata.
>>
>> Did you just do that because it seems logical?
>
> I actually was looking at moving init so runtime code and data is
> closer.
>
>> Because it does seem logical, but it leaves a RWX region in the gap
>> between __end_rodata and __init_begin, which is bad.
>>
>> This is the current behaviour, on radix:
>>
>> ---[ Start of kernel VM ]---
>> 0xc000000000000000-0xc000000001ffffff  0x0000000000000000        32M         r      X   pte  valid  present        dirty  accessed
>> 0xc000000002000000-0xc00000007fffffff  0x0000000002000000      2016M         r  w       pte  valid  present        dirty  accessed
>>
>> And with your change:
>>
>> ---[ Start of kernel VM ]---
>> 0xc000000000000000-0xc0000000013fffff  0x0000000000000000        20M         r      X   pte  valid  present        dirty  accessed
>> 0xc000000001400000-0xc000000001ffffff  0x0000000001400000        12M         r  w   X   pte  valid  present        dirty  accessed
>> 0xc000000002000000-0xc00000007fffffff  0x0000000002000000      2016M         r  w       pte  valid  present        dirty  accessed
>>
>>
>> On radix the 16M alignment is larger than we need, but we need to chose
>> a value at build time that works for radix and hash.
>>
>> We could make the code smarter on radix, to mark those pages in between
>> __end_rodata and __init_begin as RW_ and use them for data. But that
>> would be a more involved change.
>
> Ah, yes Christophe pointed out it's broken too. We could just align
> __end_rodata to STRICT_ALIGN_SIZE for this patch?

Yeah that should work.

I'd be happier if we had something more explicit to document that
boundary, I'll send a patch.

cheers