| Message ID | 20230225235642.38880-1-marmarek@invisiblethingslab.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Drop ELF notes from non-EFI binary too | expand |
On 26.02.2023 00:56, Marek Marczykowski-Górecki wrote: > The ELF is repacked from from 64bit to 32bit. With CET-related notes, > which use 64bit fields, this results in 32bit binary with corrupted > notes. Drop them all (except build-id and PVH note retained > explicitly). > > Suggested-by: Jan Beulich <jbeulich@suse.com> Perhaps a misunderstanding: Yes, I did suggest this as a possibility, but I didn't really mean we actually do so. At the very least not without further clarifying what the cons of doing so are. The notes, after all, are actually valid in xen-syms; they become bogus in the course of mkelf32's processing. > --- a/xen/arch/x86/xen.lds.S > +++ b/xen/arch/x86/xen.lds.S > @@ -192,13 +192,6 @@ SECTIONS > #endif > #endif > > -#ifndef EFI > - /* Retain these just for the purpose of possible analysis tools. */ > - DECL_SECTION(.note) { > - *(.note.*) > - } PHDR(note) PHDR(text) > -#endif > - > _erodata = .; > > . = ALIGN(SECTION_ALIGN); Is this sufficient? .note.* isn't part of DISCARD_SECTIONS except for xen.efi. I would expect it needs to move there from DISCARD_EFI_SECTIONS. Otherwise, aiui, the linker's orphan section placement will kick in. Yet at that point you'd also affect Arm and RISC-V (which, interestingly, don't place .note.* anywhere at all right now, afaics). Jan
On Mon, Feb 27, 2023 at 11:28:28AM +0100, Jan Beulich wrote: > On 26.02.2023 00:56, Marek Marczykowski-Górecki wrote: > > The ELF is repacked from from 64bit to 32bit. With CET-related notes, > > which use 64bit fields, this results in 32bit binary with corrupted > > notes. Drop them all (except build-id and PVH note retained > > explicitly). > > > > Suggested-by: Jan Beulich <jbeulich@suse.com> > > Perhaps a misunderstanding: Yes, I did suggest this as a possibility, > but I didn't really mean we actually do so. At the very least not > without further clarifying what the cons of doing so are. The notes, > after all, are actually valid in xen-syms; they become bogus in the > course of mkelf32's processing. > > > --- a/xen/arch/x86/xen.lds.S > > +++ b/xen/arch/x86/xen.lds.S > > @@ -192,13 +192,6 @@ SECTIONS > > #endif > > #endif > > > > -#ifndef EFI > > - /* Retain these just for the purpose of possible analysis tools. */ > > - DECL_SECTION(.note) { > > - *(.note.*) > > - } PHDR(note) PHDR(text) > > -#endif > > - > > _erodata = .; > > > > . = ALIGN(SECTION_ALIGN); > > Is this sufficient? .note.* isn't part of DISCARD_SECTIONS except for > xen.efi. I would expect it needs to move there from DISCARD_EFI_SECTIONS. > Otherwise, aiui, the linker's orphan section placement will kick in. What supposedly happens then? By looking at binary produced with this patch, I don't see other .note sections included. > Yet > at that point you'd also affect Arm and RISC-V (which, interestingly, > don't place .note.* anywhere at all right now, afaics). That's interesting observation. For RISC-V, I'm not surprised given how fresh it is in tree, but if Arm doesn't need it either, maybe adding to DISCARD_SECTIONS isn't such a bad idea?
On 14.03.2023 02:46, Marek Marczykowski-Górecki wrote: > On Mon, Feb 27, 2023 at 11:28:28AM +0100, Jan Beulich wrote: >> On 26.02.2023 00:56, Marek Marczykowski-Górecki wrote: >>> The ELF is repacked from from 64bit to 32bit. With CET-related notes, >>> which use 64bit fields, this results in 32bit binary with corrupted >>> notes. Drop them all (except build-id and PVH note retained >>> explicitly). >>> >>> Suggested-by: Jan Beulich <jbeulich@suse.com> >> >> Perhaps a misunderstanding: Yes, I did suggest this as a possibility, >> but I didn't really mean we actually do so. At the very least not >> without further clarifying what the cons of doing so are. The notes, >> after all, are actually valid in xen-syms; they become bogus in the >> course of mkelf32's processing. >> >>> --- a/xen/arch/x86/xen.lds.S >>> +++ b/xen/arch/x86/xen.lds.S >>> @@ -192,13 +192,6 @@ SECTIONS >>> #endif >>> #endif >>> >>> -#ifndef EFI >>> - /* Retain these just for the purpose of possible analysis tools. */ >>> - DECL_SECTION(.note) { >>> - *(.note.*) >>> - } PHDR(note) PHDR(text) >>> -#endif >>> - >>> _erodata = .; >>> >>> . = ALIGN(SECTION_ALIGN); >> >> Is this sufficient? .note.* isn't part of DISCARD_SECTIONS except for >> xen.efi. I would expect it needs to move there from DISCARD_EFI_SECTIONS. >> Otherwise, aiui, the linker's orphan section placement will kick in. > > What supposedly happens then? By looking at binary produced with this > patch, I don't see other .note sections included. The linker can't really discard them without being told so, from all I know. So the pieces must land somewhere, and considering the special section type (SHT_NOTE) I would find it odd if they were folded into some other section. >> Yet >> at that point you'd also affect Arm and RISC-V (which, interestingly, >> don't place .note.* anywhere at all right now, afaics). > > That's interesting observation. For RISC-V, I'm not surprised given how > fresh it is in tree, but if Arm doesn't need it either, maybe adding to > DISCARD_SECTIONS isn't such a bad idea? Well, yes, if we want to get rid of them, putting them there makes sense. First we need to figure where the notes end up when not placed explicitly (as that'll tell us whether on Arm they can be useful at all right now). Jan
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S index 8930e14fc40e..f0831bd677e7 100644 --- a/xen/arch/x86/xen.lds.S +++ b/xen/arch/x86/xen.lds.S @@ -192,13 +192,6 @@ SECTIONS #endif #endif -#ifndef EFI - /* Retain these just for the purpose of possible analysis tools. */ - DECL_SECTION(.note) { - *(.note.*) - } PHDR(note) PHDR(text) -#endif - _erodata = .; . = ALIGN(SECTION_ALIGN);
The ELF is repacked from from 64bit to 32bit. With CET-related notes, which use 64bit fields, this results in 32bit binary with corrupted notes. Drop them all (except build-id and PVH note retained explicitly). Suggested-by: Jan Beulich <jbeulich@suse.com> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- xen/arch/x86/xen.lds.S | 7 ------- 1 file changed, 7 deletions(-)