[PATCH] Drop ELF notes from non-EFI binary too

[PATCH] Drop ELF notes from non-EFI binary too

[Marek Marczykowski-Górecki]

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(-)

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);
-- 
2.39.2

Re: [PATCH] Drop ELF notes from non-EFI binary too

[Jan Beulich]

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

Re: [PATCH] Drop ELF notes from non-EFI binary too

[Marek Marczykowski-Górecki]

[-- Attachment #1: Type: text/plain, Size: 1890 bytes --]

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?

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] Drop ELF notes from non-EFI binary too

[Jan Beulich]

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