This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


* 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.

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

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.

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.

Thanks,
Florian


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]