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]


On Fri, Sep 21, 2018 at 7:49 AM, Florian Weimer <fweimer@redhat.com> wrote:
> * H. J. Lu:
>
>>> As far as I can tell, this patch catches only the case where a single
>>> PT_NOTE segment contains multiple notes.  If there are multiple PT_NOTE
>>> segments with notes, and each one contains a single note, the patch will
>>> still enable CET.
>>
>> I believe this is covered by CET smoke test:
>>
>> https://github.com/hjl-tools/cet-smoke-test
>>
>> My patch should work in all cases.  If it misses some cases, please add it
>> to master branch at
>>
>> https://github.com/hjl-tools/cet-smoke-test
>>
>> under the "note" directory.
>>
>>> Is my summary accurate?  Do you think it is safe to treat PT_NOTE
>>> segments in isolation?
>>
>> What do you mean by that?  The only assumption I made is that
>> property note must follow the spec.  Any violation makes it invalid.
>
> I'm attaching a patched executable which has two PT_NOTE segments with
> separately valid GNU property notes (if I did my patching correctly).
>
> I think your code will still accept it.  It is conceivable that a linker
> which is not aware of the required merging for property notes would
> create such segments, I think.

My goal is to allow older linkers which don't understand property notes
to generate working executables on CET enabled OS.  It isn't tended to
handle all hand-crafted binaries.  The worst case is that such binaries
won't run on CET enabled OS.  I don't believe it is an issue.  It may even
be a bonus.

Do you have a testcase which may be generated by an old linker on
CET enabled OS?

>>> If yes, this should be mentioned in the commit message.
>>>
>>> This seems to be an unrelated change?
>>>
>>> +             /* Property type must be in ascending order.  */
>>> +             if (type < last_type)
>>> +               return;
>>>
>>
>> This is the part of property spec.  If properties aren't properly
>> sorted, it is invalid.
>
> Okay, that part makes sense because it's internal to the notes AFAICS.
> And note merging without GNU property note awareness will likely violate
> this constraint.  Maybe you could add a comment to this effect?

I updated comments to

              /* Property types must be sorted in ascending order.
                 Ignore property note with the wrong type order.  */
              if (type < last_type)
                return;

Thanks.

-- 
H.J.


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