V2 [PATCH] Check multiple NT_GNU_PROPERTY_TYPE_0 notes [BZ #23509]

[H.J. Lu]

On Thu, Nov 8, 2018 at 5:15 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Thu, Nov 8, 2018 at 2:52 AM Florian Weimer <fweimer@redhat.com> wrote:
> >
> > * H. J. Lu:
> >
> > > On Wed, Nov 7, 2018 at 10:32 AM Florian Weimer <fweimer@redhat.com> wrote:
> > >>
> > >> * H. J. Lu:
> > >>
> > >> > The older linker treats .note.gnu.property section as a generic note
> > >> > section and just concatenates all .note.gnu.property sections from the
> > >> > inputs to the output.  When the older linker is used to created the
> > >> > program on CET-enabled OS, the generated output has .note.gnu.property
> > >> > section with multiple NT_GNU_PROPERTY_TYPE_0 notes whose IBT and SHSTK
> > >> > enable bits are set even if the program isn't CET enabled.  Such program
> > >> > will crash on CET-enabled machines.  This patch updates the note parser:
> > >> >
> > >> > 1. Skip note parsing if a NT_GNU_PROPERTY_TYPE_0 note has been processed.
> > >> > 2. Check multiple NT_GNU_PROPERTY_TYPE_0 notes.
> > >> >
> > >> >       [BZ #23509]
> > >> >       * sysdeps/x86/dl-prop.h (_dl_process_cet_property_note): Skip
> > >> >       note parsing if a NT_GNU_PROPERTY_TYPE_0 note has been processed.
> > >> >       Update the l_cet field when processing NT_GNU_PROPERTY_TYPE_0 note.
> > >> >       Check multiple NT_GNU_PROPERTY_TYPE_0 notes.
> > >> >       * sysdeps/x86/link_map.h (l_cet): Expand to 3 bits,  Add
> > >> >       lc_unknown.
> > >>
> > >> I would like to move this forward.
> > >>
> > >> I think the code is okay, as long as we are confident that older linkers
> > >> will not produce multiple PT_NOTE segments, each containing one
> > >> NT_GNU_PROPERTY_TYPE_0 note.  Maybe this should be mentioned in the
> > >> commit message (assuming that you agree that my understanding of the
> > >> code is correct).
> > >>
> > >
> > > Like this?
> >
> > Sorry, let me rephrase:
> >
> > The commit message talks about sections, not segments.  The code deals
> > with PT_NOTE segments, obviously.
>
> See my reply before.
>
> > Old linkers produce multiple PT_NOTE segments for the *same* alignment
> > if they process multiple note sections with differing alignment in a
> > certain order.  (It does not happen always.)
>
> True.
>
> > Based on reading the code, I think that if the first PT_NOTE segment it
> > encounters contains one NT_GNU_PROPERTY_TYPE_0 note that enables CET, it
> > will enable CET even if the loader produced multiple PT_NOTE segments,
> > and there is another (unmerged) NT_GNU_PROPERTY_TYPE_0 note in a
> > subsequent PT_NOTE segment.  This means that the current code can still
> > incorrectly enable CET in cases where an old linker produced a multitude
> > of PT_NOTE sections.
>
> Linkers group input note sections with the same name into a single output note
> section with the same name.  One output note section is placed in one PT_NOTE
> segment.  There won't be "another (unmerged) NT_GNU_PROPERTY_TYPE_0
> note in a subsequent PT_NOTE segment."
>
> > Based on the patch and the discussion so far it is unclear to me whether
> > you have analyzed the issue and concluded that split PT_NOTE segments of
> > the same alignment (as generated by old linkers in some cases) are not
> > relevant, or if it is an accidental omission in the patch.  The
> > explaination in the patch itself does not help because of the
> > section/segment discrepancy I mentioned first.
> >

Here is the updated patch with

Linkers group input note sections with the same name into one output
note section with the same name.  One output note section is placed in
one PT_NOTE segment.  Since new linkers merge input .note.gnu.property
sections into one output .note.gnu.property section, there is only
one NT_GNU_PROPERTY_TYPE_0 note in one PT_NOTE segment with new linkers.
Since older linkers treat input .note.gnu.property section as a generic
note section and just concatenate all input .note.gnu.property sections
into one output .note.gnu.property section without merging them, we may
see multiple NT_GNU_PROPERTY_TYPE_0 notes in one PT_NOTE segment with
older linkers.


-- 
H.J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Check-multiple-NT_GNU_PROPERTY_TYPE_0-notes-BZ-23509.patch
Type: text/x-patch
Size: 5703 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/libc-alpha/attachments/20181108/13523089/attachment.bin>