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

[Florian Weimer <fweimer at redhat dot com>]

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

>> 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?

Thanks,
Florian

Attachment: patched
Description: Patched ELF file with multiple GNU property PT_NOTE segments