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

[Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>]

On 17/03/2022 09:17, Jan Beulich wrote:
> 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.

So can I take this as an ack with the .init typo fixed?

~Andrew