[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH] x86/cet: Remove writeable mapping of the BSPs shadow stack
On 17.03.2022 17:19, Andrew Cooper wrote: > 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? R-b, yes, as long as the ALIGN(STACK_SIZE) is also dropped and the other ALIGN() is retained (the latter you did already indicate you would do). Jan
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |