| Message ID | 20240919080021.20155-4-frediano.ziglio@cloud.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | x86: Satisfy requirements for UEFI CA memory mitigation requirements | expand |
On 19.09.2024 10:00, Frediano Ziglio wrote:
> All loadable sections should be page aligned.
What about .buildid? .reloc otoh is discardable, and hence presumably okay
if mis-aligned.
Jan
On Mon, Sep 23, 2024 at 4:54 PM Jan Beulich <jbeulich@suse.com> wrote: > > On 19.09.2024 10:00, Frediano Ziglio wrote: > > All loadable sections should be page aligned. > > What about .buildid? .reloc otoh is discardable, and hence presumably okay > if mis-aligned. > > Jan Currently, internally we have a patch to make ".reloc" not discardaeble. The problem is that in case of direct EFI loading, that section is used to relocated back to the final location. On bootloaders discarding that section, you'll get a crash :-( Isn't ".buildid" a kind of subsection with the same attributes of container section? Maybe I have BUILD_ID_EFI not defined? Frediano
On 23.09.2024 18:06, Frediano Ziglio wrote: > On Mon, Sep 23, 2024 at 4:54 PM Jan Beulich <jbeulich@suse.com> wrote: >> >> On 19.09.2024 10:00, Frediano Ziglio wrote: >>> All loadable sections should be page aligned. >> >> What about .buildid? .reloc otoh is discardable, and hence presumably okay >> if mis-aligned. > > Currently, internally we have a patch to make ".reloc" not discardaeble. > The problem is that in case of direct EFI loading, that section is > used to relocated back to the final location. On bootloaders > discarding that section, you'll get a crash :-( Indeed, if such EFI loaders exist we have an issue (I don't think we actively mark the section discardable, I think that's something the linker decides). > Isn't ".buildid" a kind of subsection with the same attributes of > container section? In ELF maybe. In the PE binary it's following directly after .rodata, meaning it typically shares its space with .rodata's last page. (Aiui in PE/COFF it is illegal for multiple sections to overlap, unlike for ELF's "segments", i.e. what the program header entries describe.) > Maybe I have BUILD_ID_EFI not defined? Possible, albeit would be odd. Jan
On Tue, Sep 24, 2024 at 9:14 AM Jan Beulich <jbeulich@suse.com> wrote: > > On 23.09.2024 18:06, Frediano Ziglio wrote: > > On Mon, Sep 23, 2024 at 4:54 PM Jan Beulich <jbeulich@suse.com> wrote: > >> > >> On 19.09.2024 10:00, Frediano Ziglio wrote: > >>> All loadable sections should be page aligned. > >> > >> What about .buildid? .reloc otoh is discardable, and hence presumably okay > >> if mis-aligned. > > > > Currently, internally we have a patch to make ".reloc" not discardaeble. > > The problem is that in case of direct EFI loading, that section is > > used to relocated back to the final location. On bootloaders > > discarding that section, you'll get a crash :-( > > Indeed, if such EFI loaders exist we have an issue (I don't think we > actively mark the section discardable, I think that's something the > linker decides). > There are indeed some oddities in the final executable(s): From "objdump -x": xen/normal/xen.efi: file format pei-x86-64 xen/normal/xen.efi architecture: i386:x86-64, flags 0x0000013b: HAS_RELOC, EXEC_P, HAS_DEBUG, HAS_SYMS, HAS_LOCALS, D_PAGED start address 0xffff82d04062bfdc ... The Data Directory Entry 0 0000000000000000 00000000 Export Directory [.edata (or where ever we found it)] Entry 1 0000000000000000 00000000 Import Directory [parts of .idata] Entry 2 0000000000000000 00000000 Resource Directory [.rsrc] Entry 3 0000000000000000 00000000 Exception Directory [.pdata] Entry 4 0000000000000000 00000000 Security Directory Entry 5 00000000009489a0 000016c0 Base Relocation Directory [.reloc] Entry 6 00000000004871c8 0000001c Debug Directory Entry 7 0000000000000000 00000000 Description Directory Entry 8 0000000000000000 00000000 Special Directory ... There is a debug directory in .buildid at 0xffff82d0404871c8 ... Sections: Idx Name Size VMA LMA File off Algn 0 .text 0018c5f6 ffff82d040200000 ffff82d040200000 00001000 2**4 CONTENTS, ALLOC, LOAD, CODE 1 .rodata 000871c8 ffff82d040400000 ffff82d040400000 0018e000 2**2 CONTENTS, ALLOC, LOAD, DATA 2 .buildid 00000035 ffff82d0404871c8 ffff82d0404871c8 002151e0 2**2 CONTENTS, ALLOC, LOAD, READONLY, DATA 3 .init.text 0004775b ffff82d040600000 ffff82d040600000 00215220 2**2 CONTENTS, ALLOC, LOAD, READONLY, CODE .... Some notes: 1- I don't understand why the debug directory points to .buildid section (I suppose that's the reason for the "There is a debug directory..." message); 2- .buildid follows .rodata (this is expected); 3- .buildid is not page aligned but the loader I tried seems to be happy with that, I think it should be aligned; 4- .rodata for some reason is not marked as READONLY, even on ELF you get the same issue. For 3) I'll add the alignment. For 1) and 4) I have no idea why. Any suggestion on how to fix are welcome > > Isn't ".buildid" a kind of subsection with the same attributes of > > container section? > > In ELF maybe. In the PE binary it's following directly after .rodata, > meaning it typically shares its space with .rodata's last page. (Aiui > in PE/COFF it is illegal for multiple sections to overlap, unlike for > ELF's "segments", i.e. what the program header entries describe.) > > > Maybe I have BUILD_ID_EFI not defined? > > Possible, albeit would be odd. > > Jan Frediano
On 24.09.2024 12:22, Frediano Ziglio wrote: > There are indeed some oddities in the final executable(s): > > From "objdump -x": > > xen/normal/xen.efi: file format pei-x86-64 > xen/normal/xen.efi > architecture: i386:x86-64, flags 0x0000013b: > HAS_RELOC, EXEC_P, HAS_DEBUG, HAS_SYMS, HAS_LOCALS, D_PAGED > start address 0xffff82d04062bfdc > ... > The Data Directory > Entry 0 0000000000000000 00000000 Export Directory [.edata (or where > ever we found it)] > Entry 1 0000000000000000 00000000 Import Directory [parts of .idata] > Entry 2 0000000000000000 00000000 Resource Directory [.rsrc] > Entry 3 0000000000000000 00000000 Exception Directory [.pdata] > Entry 4 0000000000000000 00000000 Security Directory > Entry 5 00000000009489a0 000016c0 Base Relocation Directory [.reloc] > Entry 6 00000000004871c8 0000001c Debug Directory > Entry 7 0000000000000000 00000000 Description Directory > Entry 8 0000000000000000 00000000 Special Directory > ... > There is a debug directory in .buildid at 0xffff82d0404871c8 > ... > Sections: > Idx Name Size VMA LMA File off Algn > 0 .text 0018c5f6 ffff82d040200000 ffff82d040200000 00001000 2**4 > CONTENTS, ALLOC, LOAD, CODE > 1 .rodata 000871c8 ffff82d040400000 ffff82d040400000 0018e000 2**2 > CONTENTS, ALLOC, LOAD, DATA > 2 .buildid 00000035 ffff82d0404871c8 ffff82d0404871c8 002151e0 2**2 > CONTENTS, ALLOC, LOAD, READONLY, DATA > 3 .init.text 0004775b ffff82d040600000 ffff82d040600000 00215220 2**2 > CONTENTS, ALLOC, LOAD, READONLY, CODE > .... > > Some notes: > 1- I don't understand why the debug directory points to .buildid section > (I suppose that's the reason for the "There is a debug directory..." message); Like in ELF final executables, in PE ones information like this should be locatable by means other than looking up sections by name and then hoping they contain what's expected. Short of program headers and dynamic tags, this is the scheme people invented to make the build ID actually locatable. If you look at how this works in our build system, you'll find that this even requires passing a COFF object to the linker (i.e. involving a little bit of trickery). > 2- .buildid follows .rodata (this is expected); > 3- .buildid is not page aligned but the loader I tried seems to be > happy with that, > I think it should be aligned; Generally it doesn't need to be. If the secure boot stuff requires it to be, then we need to live with that (and the wasted page). Preferably it could continue to use (in the common case) a few bytes on the last .rodata page. Or we could see whether folding .buildid into .rodata works (I don't recall whether I tried that back at the time). > 4- .rodata for some reason is not marked as READONLY, even on ELF you > get the same issue. I can confirm this oddity, without having an explanation. It must be one of the inputs; I've checked that prelink.o's .rodata is r/o. So it can only be some other constituent. Jan
On 24.09.2024 13:09, Jan Beulich wrote: > On 24.09.2024 12:22, Frediano Ziglio wrote: >> 4- .rodata for some reason is not marked as READONLY, even on ELF you >> get the same issue. > > I can confirm this oddity, without having an explanation. It must be > one of the inputs; I've checked that prelink.o's .rodata is r/o. So it > can only be some other constituent. That's from .data.ro_after_init and .data.rel.ro*. Jan
On Tue, Sep 24, 2024 at 1:17 PM Jan Beulich <jbeulich@suse.com> wrote: > > On 24.09.2024 13:09, Jan Beulich wrote: > > On 24.09.2024 12:22, Frediano Ziglio wrote: > >> 4- .rodata for some reason is not marked as READONLY, even on ELF you > >> get the same issue. > > > > I can confirm this oddity, without having an explanation. It must be > > one of the inputs; I've checked that prelink.o's .rodata is r/o. So it > > can only be some other constituent. > > That's from .data.ro_after_init and .data.rel.ro*. > > Jan That makes sense. On a similar note, what about .text? I mean, all sections are READONLY (or at least from mapfile) but .text is not marked as READONLY. Frediano
On 24.09.2024 14:22, Frediano Ziglio wrote: > On Tue, Sep 24, 2024 at 1:17 PM Jan Beulich <jbeulich@suse.com> wrote: >> >> On 24.09.2024 13:09, Jan Beulich wrote: >>> On 24.09.2024 12:22, Frediano Ziglio wrote: >>>> 4- .rodata for some reason is not marked as READONLY, even on ELF you >>>> get the same issue. >>> >>> I can confirm this oddity, without having an explanation. It must be >>> one of the inputs; I've checked that prelink.o's .rodata is r/o. So it >>> can only be some other constituent. >> >> That's from .data.ro_after_init and .data.rel.ro*. > > That makes sense. > On a similar note, what about .text? I mean, all sections are READONLY > (or at least from mapfile) but .text is not marked as READONLY. Can't spot anything for now. Jan
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S index b0b952dd9c..ef446e0a71 100644 --- a/xen/arch/x86/xen.lds.S +++ b/xen/arch/x86/xen.lds.S @@ -208,6 +208,10 @@ SECTIONS } PHDR(text) +#ifdef EFI + /* align to satisfy UEFI CA memory mitigation */ + . = ALIGN(PAGE_SIZE); +#endif DECL_SECTION(.init.data) { *(.init.bss.stack_aligned) @@ -262,6 +266,10 @@ SECTIONS __ctors_end = .; } PHDR(text) +#ifdef EFI + /* align to satisfy UEFI CA memory mitigation */ + . = ALIGN(PAGE_SIZE); +#endif DECL_SECTION(.init.trampoline) { *(.init.trampoline) } PHDR(text)
All loadable sections should be page aligned. Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com> --- xen/arch/x86/xen.lds.S | 8 ++++++++ 1 file changed, 8 insertions(+)