Xen-Devel Archive mirror
 help / color / mirror / Atom feed
* [PATCH] Drop ELF notes from non-EFI binary too
@ 2023-02-25 23:56 Marek Marczykowski-Górecki
  2023-02-27 10:28 ` Jan Beulich
  0 siblings, 1 reply; 4+ messages in thread
From: Marek Marczykowski-Górecki @ 2023-02-25 23:56 UTC (permalink / raw
  To: xen-devel
  Cc: Marek Marczykowski-Górecki, Jan Beulich, Andrew Cooper,
	Roger Pau Monné, Wei Liu

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



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] Drop ELF notes from non-EFI binary too
  2023-02-25 23:56 [PATCH] Drop ELF notes from non-EFI binary too Marek Marczykowski-Górecki
@ 2023-02-27 10:28 ` Jan Beulich
  2023-03-14  1:46   ` Marek Marczykowski-Górecki
  0 siblings, 1 reply; 4+ messages in thread
From: Jan Beulich @ 2023-02-27 10:28 UTC (permalink / raw
  To: Marek Marczykowski-Górecki
  Cc: Andrew Cooper, Roger Pau Monné, Wei Liu, xen-devel

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Drop ELF notes from non-EFI binary too
  2023-02-27 10:28 ` Jan Beulich
@ 2023-03-14  1:46   ` Marek Marczykowski-Górecki
  2023-03-14  6:30     ` Jan Beulich
  0 siblings, 1 reply; 4+ messages in thread
From: Marek Marczykowski-Górecki @ 2023-03-14  1:46 UTC (permalink / raw
  To: Jan Beulich; +Cc: Andrew Cooper, Roger Pau Monné, Wei Liu, xen-devel

[-- 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 --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Drop ELF notes from non-EFI binary too
  2023-03-14  1:46   ` Marek Marczykowski-Górecki
@ 2023-03-14  6:30     ` Jan Beulich
  0 siblings, 0 replies; 4+ messages in thread
From: Jan Beulich @ 2023-03-14  6:30 UTC (permalink / raw
  To: Marek Marczykowski-Górecki
  Cc: Andrew Cooper, Roger Pau Monné, Wei Liu, xen-devel

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2023-03-14  6:31 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-25 23:56 [PATCH] Drop ELF notes from non-EFI binary too Marek Marczykowski-Górecki
2023-02-27 10:28 ` Jan Beulich
2023-03-14  1:46   ` Marek Marczykowski-Górecki
2023-03-14  6:30     ` Jan Beulich

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).