This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: V2 [PATCH] Check multiple NT_GNU_PROPERTY_TYPE_0 notes [BZ #23509]
- From: "H.J. Lu" <hjl dot tools at gmail dot com>
- To: Florian Weimer <fweimer at redhat dot com>
- Cc: GNU C Library <libc-alpha at sourceware dot org>
- Date: Fri, 21 Sep 2018 08:31:26 -0700
- Subject: Re: V2 [PATCH] Check multiple NT_GNU_PROPERTY_TYPE_0 notes [BZ #23509]
- References: <CAMe9rOoXP7fz+XaM5hFnzDa=qSzz=8r3HkoddaTscrh7HUt03g@mail.gmail.com> <87a7ocmtvd.fsf@oldenburg.str.redhat.com> <CAMe9rOoAXzKfS0Ce=CF+pZhyQ2-Pzq6U+5a-pNx9M2fEKE-Vcg@mail.gmail.com> <871s9mdhy8.fsf@oldenburg.str.redhat.com>
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.