Re: [PATCH] x86/cet: Remove writeable mapping of the BSPs shadow stack

[Jan Beulich <jbeulich@xxxxxxxx>]

On 16.03.2022 20:23, Andrew Cooper wrote:
> On 16/03/2022 08:33, Jan Beulich wrote:
>> On 15.03.2022 17:53, Andrew Cooper wrote:
>>> --- a/xen/arch/x86/xen.lds.S
>>> +++ b/xen/arch/x86/xen.lds.S
>>> @@ -215,8 +215,9 @@ SECTIONS
>>>    } PHDR(text)
>>>    DECL_SECTION(.init.data) {
>>>  #endif
>>> +       . = ALIGN(STACK_SIZE);
>>> +       *(.init.bss.stack_aligned)
>> No real need for the ALIGN() here - it's the contributions to the
>> section which ought to come with proper alignment. Imo ALIGN()
>> should only ever be there ahead of a symbol definition, as otherwise
>> the symbol might not mark what it is intended to mark due to padding
>> which might be inserted. See also 01fe4da6243b ("x86: force suitable
>> alignment in sources rather than in linker script").
>>
>> Really we should consider using
>>
>>     *(SORT_BY_ALIGNMENT(.init.data .init.data.* .init.bss.*))
>>
>> While I can see your point against forcing sorting by alignment
>> globally, this very argument doesn't apply here (at least until
>> there appeared a way for the section attribute and -fdata-sections
>> to actually interact, such that .init.* could also become per-
>> function/object).
>>
>> Then again - this block of zeroes doesn't need to occupy space in
>> the binary.
> 
> It already occupies space, because of mkelf32.

Hmm, yes, and not just because of mkelf32: Since we munge everything
in a single PT_LOAD segment in the linker script, all of .init.*
necessarily has space allocated.

>>  It could very well live in a @nobits .init.bss in the
>> final ELF binary. But sadly the section isn't @nobits in the object
>> file, and with that there would need to be a way to make the linker
>> convert it to @nobits (and I'm unaware of such). What would work is
>> naming the section .bss.init.stack_aligned (or e.g.
>> .bss..init.stack_aligned to make it easier to separate it from
>> .bss.* in the linker script) - that'll make gcc mark it @nobits.
> 
> Living in .bss would prevent it from being reclaimed.  We've got several
> nasty bugs from shooting holes in the Xen image, and too many special
> cases already.

I didn't suggest to put it in .bss; the suggested name was merely so
that gcc would mark the section @nobits and we could exclude the
section from what makes in into .bss in the final image independent
of .init.* vs .bss.* ordering in the linker script. But anyway - with
the above this aspect is now moot anyway.

Jan