diff mbox

Check multiple NT_GNU_PROPERTY_TYPE_0 notes [BZ #23509]

Message ID 20180814173238.GA8590@intel.com
State New, archived
Headers

Commit Message

Lu, Hongjiu Aug. 14, 2018, 5:32 p.m. UTC 
  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.

OK for master?

H.J.
---
	[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.
---
 sysdeps/x86/dl-prop.h  | 51 +++++++++++++++++++++++++++++++++---------
 sysdeps/x86/link_map.h |  9 ++++----
 2 files changed, 46 insertions(+), 14 deletions(-)

Comments

Florian Weimer Aug. 14, 2018, 5:59 p.m. UTC | #1 
On 08/14/2018 07:32 PM, H.J. Lu wrote:

>   #if CET_ENABLED
> +  /* Skip if we have seen a NT_GNU_PROPERTY_TYPE_0 note before.  */
> +  if (l->l_cet != lc_unknown)
> +    return;

Should this set l->l_cet to lc_none before returning?

> +	  /* Stop if we see more than one GNU property note which may
> +	     be generated by the older linker.  */
> +	  if (l->l_cet != lc_unknown)
> +	    return;

Likewise.

Thanks,
Florian
H.J. Lu Aug. 14, 2018, 6:12 p.m. UTC | #2 
On Tue, Aug 14, 2018 at 10:59 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 08/14/2018 07:32 PM, H.J. Lu wrote:
>
>>   #if CET_ENABLED
>> +  /* Skip if we have seen a NT_GNU_PROPERTY_TYPE_0 note before.  */
>> +  if (l->l_cet != lc_unknown)
>> +    return;
>
>
> Should this set l->l_cet to lc_none before returning?
>
>> +         /* Stop if we see more than one GNU property note which may
>> +            be generated by the older linker.  */
>> +         if (l->l_cet != lc_unknown)
>> +           return;
>
>
> Likewise.
>

_dl_process_cet_property_note is called on on each PT_NOTE segment.
We must keep searching until we found a PT_NOTE segment with a
NT_GNU_PROPERTY_TYPE_0 note:

  /* Skip if we have seen a NT_GNU_PROPERTY_TYPE_0 note before.  */
  if (l->l_cet != lc_unknown)
    return;
...

<<< Found a PT_NOTE segment, which may not contain
the NT_GNU_PROPERTY_TYPE_0 note.

     /* Find the NT_GNU_PROPERTY_TYPE_0 note.  */
      if (note->n_namesz == 4
          && note->n_type == NT_GNU_PROPERTY_TYPE_0
          && memcmp (note + 1, "GNU", 4) == 0)
        {

<<< Now we found a NT_GNU_PROPERTY_TYPE_0 note.

          /* Stop if we see more than one GNU property note which may
             be generated by the older linker.  */
          if (l->l_cet != lc_unknown)  <<<<<<< l_cet starts with lc_unknown.
            return;

          /* Check CET status now.  */
          l->l_cet = lc_none; <<<<<<<<<  Set l_cet to lc_none now.

l->l_cet may be left as lc_unknown if there is no NT_GNU_PROPERTY_TYPE_0
note.
Florian Weimer Aug. 15, 2018, 3:17 p.m. UTC | #3 
On 08/14/2018 08:12 PM, H.J. Lu wrote:
> _dl_process_cet_property_note is called on on each PT_NOTE segment.
> We must keep searching until we found a PT_NOTE segment with a
> NT_GNU_PROPERTY_TYPE_0 note:
> 
>    /* Skip if we have seen a NT_GNU_PROPERTY_TYPE_0 note before.  */
>    if (l->l_cet != lc_unknown)
>      return;
> ...

What if there are different segments, each one with its own 
NT_GNU_PROPERTY_TYPE_0?  Wouldn't that point to lack of linker support, 
too?  So that CET needs to be disabled?

Thanks,
Florian
H.J. Lu Aug. 15, 2018, 3:23 p.m. UTC | #4 
On Wed, Aug 15, 2018 at 8:17 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 08/14/2018 08:12 PM, H.J. Lu wrote:
>>
>> _dl_process_cet_property_note is called on on each PT_NOTE segment.
>> We must keep searching until we found a PT_NOTE segment with a
>> NT_GNU_PROPERTY_TYPE_0 note:
>>
>>    /* Skip if we have seen a NT_GNU_PROPERTY_TYPE_0 note before.  */
>>    if (l->l_cet != lc_unknown)
>>      return;
>> ...
>
>
> What if there are different segments, each one with its own
> NT_GNU_PROPERTY_TYPE_0?  Wouldn't that point to lack of linker support, too?
> So that CET needs to be disabled?

Older linkers puts all NT_GNU_PROPERTY_TYPE_0 notes in one
.note.gnu.property section.  One can certainly create some random notes
which look like NT_GNU_PROPERTY_TYPE_0.  I don't think we need to
check these notes.
Florian Weimer Aug. 15, 2018, 3:25 p.m. UTC | #5 
On 08/15/2018 05:23 PM, H.J. Lu wrote:
> On Wed, Aug 15, 2018 at 8:17 AM, Florian Weimer <fweimer@redhat.com> wrote:
>> On 08/14/2018 08:12 PM, H.J. Lu wrote:
>>>
>>> _dl_process_cet_property_note is called on on each PT_NOTE segment.
>>> We must keep searching until we found a PT_NOTE segment with a
>>> NT_GNU_PROPERTY_TYPE_0 note:
>>>
>>>     /* Skip if we have seen a NT_GNU_PROPERTY_TYPE_0 note before.  */
>>>     if (l->l_cet != lc_unknown)
>>>       return;
>>> ...
>>
>>
>> What if there are different segments, each one with its own
>> NT_GNU_PROPERTY_TYPE_0?  Wouldn't that point to lack of linker support, too?
>> So that CET needs to be disabled?
> 
> Older linkers puts all NT_GNU_PROPERTY_TYPE_0 notes in one
> .note.gnu.property section.  One can certainly create some random notes
> which look like NT_GNU_PROPERTY_TYPE_0.  I don't think we need to
> check these notes.

I think older BFD ld creates multiple segments.

Thanks,
Florian
H.J. Lu Aug. 15, 2018, 3:29 p.m. UTC | #6 
On Wed, Aug 15, 2018 at 8:25 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 08/15/2018 05:23 PM, H.J. Lu wrote:
>>
>> On Wed, Aug 15, 2018 at 8:17 AM, Florian Weimer <fweimer@redhat.com>
>> wrote:
>>>
>>> On 08/14/2018 08:12 PM, H.J. Lu wrote:
>>>>
>>>>
>>>> _dl_process_cet_property_note is called on on each PT_NOTE segment.
>>>> We must keep searching until we found a PT_NOTE segment with a
>>>> NT_GNU_PROPERTY_TYPE_0 note:
>>>>
>>>>     /* Skip if we have seen a NT_GNU_PROPERTY_TYPE_0 note before.  */
>>>>     if (l->l_cet != lc_unknown)
>>>>       return;
>>>> ...
>>>
>>>
>>>
>>> What if there are different segments, each one with its own
>>> NT_GNU_PROPERTY_TYPE_0?  Wouldn't that point to lack of linker support,
>>> too?
>>> So that CET needs to be disabled?
>>
>>
>> Older linkers puts all NT_GNU_PROPERTY_TYPE_0 notes in one
>> .note.gnu.property section.  One can certainly create some random notes
>> which look like NT_GNU_PROPERTY_TYPE_0.  I don't think we need to
>> check these notes.
>
>
> I think older BFD ld creates multiple segments.

Which version of ld dos that?
Florian Weimer Aug. 16, 2018, 11:28 a.m. UTC | #7 
On 08/15/2018 05:29 PM, H.J. Lu wrote:
> On Wed, Aug 15, 2018 at 8:25 AM, Florian Weimer <fweimer@redhat.com> wrote:
>> On 08/15/2018 05:23 PM, H.J. Lu wrote:
>>>
>>> On Wed, Aug 15, 2018 at 8:17 AM, Florian Weimer <fweimer@redhat.com>
>>> wrote:
>>>>
>>>> On 08/14/2018 08:12 PM, H.J. Lu wrote:
>>>>>
>>>>>
>>>>> _dl_process_cet_property_note is called on on each PT_NOTE segment.
>>>>> We must keep searching until we found a PT_NOTE segment with a
>>>>> NT_GNU_PROPERTY_TYPE_0 note:
>>>>>
>>>>>      /* Skip if we have seen a NT_GNU_PROPERTY_TYPE_0 note before.  */
>>>>>      if (l->l_cet != lc_unknown)
>>>>>        return;
>>>>> ...
>>>>
>>>>
>>>>
>>>> What if there are different segments, each one with its own
>>>> NT_GNU_PROPERTY_TYPE_0?  Wouldn't that point to lack of linker support,
>>>> too?
>>>> So that CET needs to be disabled?
>>>
>>>
>>> Older linkers puts all NT_GNU_PROPERTY_TYPE_0 notes in one
>>> .note.gnu.property section.  One can certainly create some random notes
>>> which look like NT_GNU_PROPERTY_TYPE_0.  I don't think we need to
>>> check these notes.
>>
>>
>> I think older BFD ld creates multiple segments.
> 
> Which version of ld dos that?

Examples that got me worried look like this:

$ readelf -l /usr/bin/b5i2iso
…
   NOTE           0x0000000000000334 0x0000000000400334 0x0000000000400334
                  0x0000000000000020 0x0000000000000020  R      0x4
   NOTE           0x0000000000000358 0x0000000000400358 0x0000000000400358
                  0x0000000000000030 0x0000000000000030  R      0x8
   NOTE           0x0000000000000388 0x0000000000400388 0x0000000000400388
                  0x0000000000000024 0x0000000000000024  R      0x4
…
    07     .note.ABI-tag
    08     .note.gnu.property
    09     .note.gnu.build-id
…
$ rpm -qf /usr/bin/b5i2iso
AcetoneISO-6.7-26.fc29.x86_64

Per 
<https://kojipkgs.fedoraproject.org//packages/AcetoneISO/6.7/26.fc29/data/logs/x86_64/root.log>, 
this was built with binutils-2.30.90-4.fc29.

I don't know enough about the binutils linker to be certain that 
allocatable SHT_NOTE sections of the same name always go into the same 
PT_NOTE segment.  In the example above, the sections weren't split.

Thanks,
Florian
H.J. Lu Aug. 16, 2018, 11:39 a.m. UTC | #8 
On Thu, Aug 16, 2018 at 4:28 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 08/15/2018 05:29 PM, H.J. Lu wrote:
>>
>> On Wed, Aug 15, 2018 at 8:25 AM, Florian Weimer <fweimer@redhat.com>
>> wrote:
>>>
>>> On 08/15/2018 05:23 PM, H.J. Lu wrote:
>>>>
>>>>
>>>> On Wed, Aug 15, 2018 at 8:17 AM, Florian Weimer <fweimer@redhat.com>
>>>> wrote:
>>>>>
>>>>>
>>>>> On 08/14/2018 08:12 PM, H.J. Lu wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> _dl_process_cet_property_note is called on on each PT_NOTE segment.
>>>>>> We must keep searching until we found a PT_NOTE segment with a
>>>>>> NT_GNU_PROPERTY_TYPE_0 note:
>>>>>>
>>>>>>      /* Skip if we have seen a NT_GNU_PROPERTY_TYPE_0 note before.  */
>>>>>>      if (l->l_cet != lc_unknown)
>>>>>>        return;
>>>>>> ...
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> What if there are different segments, each one with its own
>>>>> NT_GNU_PROPERTY_TYPE_0?  Wouldn't that point to lack of linker support,
>>>>> too?
>>>>> So that CET needs to be disabled?
>>>>
>>>>
>>>>
>>>> Older linkers puts all NT_GNU_PROPERTY_TYPE_0 notes in one
>>>> .note.gnu.property section.  One can certainly create some random notes
>>>> which look like NT_GNU_PROPERTY_TYPE_0.  I don't think we need to
>>>> check these notes.
>>>
>>>
>>>
>>> I think older BFD ld creates multiple segments.
>>
>>
>> Which version of ld dos that?
>
>
> Examples that got me worried look like this:
>
> $ readelf -l /usr/bin/b5i2iso
> …
>   NOTE           0x0000000000000334 0x0000000000400334 0x0000000000400334
>                  0x0000000000000020 0x0000000000000020  R      0x4
>   NOTE           0x0000000000000358 0x0000000000400358 0x0000000000400358
>                  0x0000000000000030 0x0000000000000030  R      0x8
>   NOTE           0x0000000000000388 0x0000000000400388 0x0000000000400388
>                  0x0000000000000024 0x0000000000000024  R      0x4
> …
>    07     .note.ABI-tag
>    08     .note.gnu.property
>    09     .note.gnu.build-id
> …
> $ rpm -qf /usr/bin/b5i2iso
> AcetoneISO-6.7-26.fc29.x86_64
>
> Per
> <https://kojipkgs.fedoraproject.org//packages/AcetoneISO/6.7/26.fc29/data/logs/x86_64/root.log>,
> this was built with binutils-2.30.90-4.fc29.
>
> I don't know enough about the binutils linker to be certain that allocatable
> SHT_NOTE sections of the same name always go into the same PT_NOTE segment.
> In the example above, the sections weren't split.

Please show the output of "readelf -n".  I believe that all
NT_GNU_PROPERTY_TYPE_0
notes are in a single PT_NOTE segment.  My patch checks each PT_LOAD segment
until a NT_GNU_PROPERTY_TYPE_0 note is found.  If more than one note is found,
it disables CET since the binary was created by the older linker.

BTW, the new linker also creates more than one note:

[hjl@gnu-cfl-1 binutils]$ readelf -ln  /bin/sprof

Elf file type is DYN (Shared object file)
Entry point 0x36d0
There are 12 program headers, starting at offset 64

Program Headers:
  Type           Offset             VirtAddr           PhysAddr
                 FileSiz            MemSiz              Flags  Align
  PHDR           0x0000000000000040 0x0000000000000040 0x0000000000000040
                 0x00000000000002a0 0x00000000000002a0  R      0x8
  INTERP         0x00000000000002e0 0x00000000000002e0 0x00000000000002e0
                 0x000000000000001c 0x000000000000001c  R      0x1
      [Requesting program interpreter: /lib64/ld-linux-x86-64.so.2]
  LOAD           0x0000000000000000 0x0000000000000000 0x0000000000000000
                 0x0000000000001708 0x0000000000001708  R      0x1000
  LOAD           0x0000000000002000 0x0000000000002000 0x0000000000002000
                 0x0000000000002fc5 0x0000000000002fc5  R E    0x1000
  LOAD           0x0000000000005000 0x0000000000005000 0x0000000000005000
                 0x0000000000000c30 0x0000000000000c30  R      0x1000
  LOAD           0x0000000000005c70 0x0000000000006c70 0x0000000000006c70
                 0x0000000000000590 0x0000000000000670  RW     0x1000
  DYNAMIC        0x0000000000005dc8 0x0000000000006dc8 0x0000000000006dc8
                 0x0000000000000200 0x0000000000000200  RW     0x8
  NOTE           0x0000000000000300 0x0000000000000300 0x0000000000000300
                 0x0000000000000020 0x0000000000000020  R      0x8
  NOTE           0x0000000000000320 0x0000000000000320 0x0000000000000320
                 0x0000000000000044 0x0000000000000044  R      0x4
  GNU_EH_FRAME   0x00000000000058f0 0x00000000000058f0 0x00000000000058f0
                 0x00000000000000ac 0x00000000000000ac  R      0x4
  GNU_STACK      0x0000000000000000 0x0000000000000000 0x0000000000000000
                 0x0000000000000000 0x0000000000000000  RW     0x10
  GNU_RELRO      0x0000000000005c70 0x0000000000006c70 0x0000000000006c70
                 0x0000000000000390 0x0000000000000390  R      0x1

 Section to Segment mapping:
  Segment Sections...
   00
   01     .interp
   02     .interp .note.gnu.property .note.ABI-tag .note.gnu.build-id
.hash .gnu.hash .dynsym .dynstr .gnu.version .gnu.version_r .rela.dyn
.rela.plt
   03     .init .plt .plt.got .plt.sec .text .fini
   04     .rodata .eh_frame_hdr .eh_frame
   05     .init_array .fini_array .data.rel.ro .dynamic .got .got.plt .data .bss
   06     .dynamic
   07     .note.gnu.property
   08     .note.ABI-tag .note.gnu.build-id
   09     .eh_frame_hdr
   10
   11     .init_array .fini_array .data.rel.ro .dynamic .got

Displaying notes found in: .note.gnu.property
  Owner                 Data size Description
  GNU                  0x00000010 NT_GNU_PROPERTY_TYPE_0
      Properties: x86 feature: IBT, SHSTK

Displaying notes found in: .note.ABI-tag
  Owner                 Data size Description
  GNU                  0x00000010 NT_GNU_ABI_TAG (ABI version tag)
    OS: Linux, ABI: 3.2.0

Displaying notes found in: .note.gnu.build-id
  Owner                 Data size Description
  GNU                  0x00000014 NT_GNU_BUILD_ID (unique build ID bitstring)
    Build ID: 59cb797230842a68fe5d61189f03c50acb9e84bf
[hjl@gnu-cfl-1 binutils]$
Florian Weimer Aug. 16, 2018, 11:47 a.m. UTC | #9 
On 08/16/2018 01:39 PM, H.J. Lu wrote:

> Please show the output of "readelf -n".  I believe that all
> NT_GNU_PROPERTY_TYPE_0
> notes are in a single PT_NOTE segment.

There is just one property note because the linker has property note 
merging support:

Displaying notes found in: .note.ABI-tag
   Owner                 Data size       Description
   GNU                  0x00000010       NT_GNU_ABI_TAG (ABI version tag)
     OS: Linux, ABI: 3.2.0

Displaying notes found in: .note.gnu.property
   Owner                 Data size       Description
   GNU                  0x00000020       NT_GNU_PROPERTY_TYPE_0
       Properties: x86 ISA used:
         x86 ISA needed:

Displaying notes found in: .note.gnu.build-id
   Owner                 Data size       Description
   GNU                  0x00000014       NT_GNU_BUILD_ID (unique build 
ID bitstring)
     Build ID: 5bd43ed7376c4734e49138a6bddb15dc0d0002a6

> My patch checks each PT_LOAD segment
> until a NT_GNU_PROPERTY_TYPE_0 note is found.  If more than one note is found,
> it disables CET since the binary was created by the older linker.

I assumed the patch stopped searching once it finds a segment with a 
single NT_GNU_PROPERTY_TYPE_0 note.  I'm still worried that this might 
be a problem, considering that ld produces multiple segments in some 
cases (but the triggering conditions for that aren't entirely clear to me).

Thanks,
Florian
H.J. Lu Aug. 16, 2018, 12:10 p.m. UTC | #10 
On Thu, Aug 16, 2018 at 4:47 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 08/16/2018 01:39 PM, H.J. Lu wrote:
>
>> Please show the output of "readelf -n".  I believe that all
>> NT_GNU_PROPERTY_TYPE_0
>> notes are in a single PT_NOTE segment.
>
>
> There is just one property note because the linker has property note merging
> support:
>
> Displaying notes found in: .note.ABI-tag
>   Owner                 Data size       Description
>   GNU                  0x00000010       NT_GNU_ABI_TAG (ABI version tag)
>     OS: Linux, ABI: 3.2.0
>
> Displaying notes found in: .note.gnu.property
>   Owner                 Data size       Description
>   GNU                  0x00000020       NT_GNU_PROPERTY_TYPE_0
>       Properties: x86 ISA used:
>         x86 ISA needed:
>
> Displaying notes found in: .note.gnu.build-id
>   Owner                 Data size       Description
>   GNU                  0x00000014       NT_GNU_BUILD_ID (unique build ID
> bitstring)
>     Build ID: 5bd43ed7376c4734e49138a6bddb15dc0d0002a6
>
>> My patch checks each PT_LOAD segment
>> until a NT_GNU_PROPERTY_TYPE_0 note is found.  If more than one note is
>> found,
>> it disables CET since the binary was created by the older linker.
>
>
> I assumed the patch stopped searching once it finds a segment with a single
> NT_GNU_PROPERTY_TYPE_0 note.  I'm still worried that this might be a
> problem, considering that ld produces multiple segments in some cases (but
> the triggering conditions for that aren't entirely clear to me).
>

Multiple segments are perfectly normal.  But there is only one
.note.gnu.property
section, which may contain more then one NT_GNU_PROPERTY_TYPE_0 notes.
This was built by the old linker:

[hjl@gnu-cet-1 pr23509]$ readelf -n foo

Displaying notes found in: .note.gnu.property
  Owner                 Data size Description
  GNU                  0x00000020 NT_GNU_PROPERTY_TYPE_0
      Properties: Invalid x86 feature: IBT, SHSTK
Invalid x86 feature used: x86
  GNU                  0x00000010 NT_GNU_PROPERTY_TYPE_0
      Properties: Invalid x86 feature: IBT, SHSTK
  GNU                  0x00000010 NT_GNU_PROPERTY_TYPE_0
      Properties: Invalid x86 feature used: x86
  GNU                  0x00000010 NT_GNU_PROPERTY_TYPE_0
      Properties: Invalid x86 feature: IBT, SHSTK
  GNU                  0x00000010 NT_GNU_PROPERTY_TYPE_0
      Properties: Invalid x86 feature used: x86
  GNU                  0x00000010 NT_GNU_PROPERTY_TYPE_0
      Properties: Invalid x86 feature: IBT, SHSTK
  GNU                  0x00000010 NT_GNU_PROPERTY_TYPE_0
      Properties: Invalid x86 feature used: x86
  GNU                  0x00000010 NT_GNU_PROPERTY_TYPE_0
      Properties: Invalid x86 feature: IBT, SHSTK
  GNU                  0x00000010 NT_GNU_PROPERTY_TYPE_0
      Properties: Invalid x86 feature: IBT, SHSTK
  GNU                  0x00000010 NT_GNU_PROPERTY_TYPE_0
      Properties: Invalid x86 feature used: x86

Displaying notes found in: .note.ABI-tag
  Owner                 Data size Description
  GNU                  0x00000010 NT_GNU_ABI_TAG (ABI version tag)
    OS: Linux, ABI: 3.2.0

Displaying notes found in: .note.gnu.build-id
  Owner                 Data size Description
  GNU                  0x00000014 NT_GNU_BUILD_ID (unique build ID bitstring)
    Build ID: e07c4ee5b30388bc95738a19cba8b9e13018176c
[hjl@gnu-cet-1 pr23509]$
Florian Weimer Aug. 16, 2018, 12:29 p.m. UTC | #11 
On 08/16/2018 02:10 PM, H.J. Lu wrote:

> Multiple segments are perfectly normal.

I'm not convinced that they are desirable.  They really should be merged.

> But there is only one
> .note.gnu.property
> section, which may contain more then one NT_GNU_PROPERTY_TYPE_0 notes.

My worry is that if the linker generates multiple note segments, it 
might as well produce multiple segments from the .note.gnu.property section.

Florian
H.J. Lu Aug. 16, 2018, 12:53 p.m. UTC | #12 
On Thu, Aug 16, 2018 at 5:29 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 08/16/2018 02:10 PM, H.J. Lu wrote:
>
>> Multiple segments are perfectly normal.
>
>
> I'm not convinced that they are desirable.  They really should be merged.

They aren't merged since they have different alignments.

>> But there is only one
>> .note.gnu.property
>> section, which may contain more then one NT_GNU_PROPERTY_TYPE_0 notes.
>
>
> My worry is that if the linker generates multiple note segments, it might as
> well produce multiple segments from the .note.gnu.property section.

Linker generates multiple segments because of different segment alignments.
But linker should always place all .note.gnu.property sections from inputs in
one output .note.gnu.property section.   You can try different older linkers.
They should all behave the same.
diff mbox

Patch

diff --git a/sysdeps/x86/dl-prop.h b/sysdeps/x86/dl-prop.h
index 26c3131ac5..d3979aec2d 100644
--- a/sysdeps/x86/dl-prop.h
+++ b/sysdeps/x86/dl-prop.h
@@ -49,6 +49,10 @@  _dl_process_cet_property_note (struct link_map *l,
 			      const ElfW(Addr) align)
 {
 #if CET_ENABLED
+  /* Skip if we have seen a NT_GNU_PROPERTY_TYPE_0 note before.  */
+  if (l->l_cet != lc_unknown)
+    return;
+
   /* The NT_GNU_PROPERTY_TYPE_0 note must be aliged to 4 bytes in
      32-bit objects and to 8 bytes in 64-bit objects.  Skip notes
      with incorrect alignment.  */
@@ -57,6 +61,9 @@  _dl_process_cet_property_note (struct link_map *l,
 
   const ElfW(Addr) start = (ElfW(Addr)) note;
 
+  unsigned int feature_1 = 0;
+  unsigned int last_type = 0;
+
   while ((ElfW(Addr)) (note + 1) - start < size)
     {
       /* Find the NT_GNU_PROPERTY_TYPE_0 note.  */
@@ -64,10 +71,18 @@  _dl_process_cet_property_note (struct link_map *l,
 	  && note->n_type == NT_GNU_PROPERTY_TYPE_0
 	  && memcmp (note + 1, "GNU", 4) == 0)
 	{
+	  /* Stop if we see more than one GNU property note which may
+	     be generated by the older linker.  */
+	  if (l->l_cet != lc_unknown)
+	    return;
+
+	  /* Check CET status now.  */
+	  l->l_cet = lc_none;
+
 	  /* Check for invalid property.  */
 	  if (note->n_descsz < 8
 	      || (note->n_descsz % sizeof (ElfW(Addr))) != 0)
-	    break;
+	    return;
 
 	  /* Start and end of property array.  */
 	  unsigned char *ptr = (unsigned char *) (note + 1) + 4;
@@ -78,9 +93,15 @@  _dl_process_cet_property_note (struct link_map *l,
 	      unsigned int type = *(unsigned int *) ptr;
 	      unsigned int datasz = *(unsigned int *) (ptr + 4);
 
+	      /* Property type must be in ascending order.  */
+	      if (type <= last_type)
+		return;
+
 	      ptr += 8;
 	      if ((ptr + datasz) > ptr_end)
-		break;
+		return;
+
+	      last_type = type;
 
 	      if (type == GNU_PROPERTY_X86_FEATURE_1_AND)
 		{
@@ -89,14 +110,18 @@  _dl_process_cet_property_note (struct link_map *l,
 		     we stop the search regardless if its size is correct
 		     or not.  There is no point to continue if this note
 		     is ill-formed.  */
-		  if (datasz == 4)
-		    {
-		      unsigned int feature_1 = *(unsigned int *) ptr;
-		      if ((feature_1 & GNU_PROPERTY_X86_FEATURE_1_IBT))
-			l->l_cet |= lc_ibt;
-		      if ((feature_1 & GNU_PROPERTY_X86_FEATURE_1_SHSTK))
-			l->l_cet |= lc_shstk;
-		    }
+		  if (datasz != 4)
+		    return;
+
+		  feature_1 = *(unsigned int *) ptr;
+
+		  /* Keep searching for the next GNU property note
+		     generated by the older linker.  */
+		  break;
+		}
+	      else if (type > GNU_PROPERTY_X86_FEATURE_1_AND)
+		{
+		  /* Stop since property type is in ascending order.  */
 		  return;
 		}
 
@@ -112,6 +137,12 @@  _dl_process_cet_property_note (struct link_map *l,
 	      + ELF_NOTE_NEXT_OFFSET (note->n_namesz, note->n_descsz,
 				      align));
     }
+
+  /* We get here only if there is one or no GNU property note.  */
+  if ((feature_1 & GNU_PROPERTY_X86_FEATURE_1_IBT))
+    l->l_cet |= lc_ibt;
+  if ((feature_1 & GNU_PROPERTY_X86_FEATURE_1_SHSTK))
+    l->l_cet |= lc_shstk;
 #endif
 }
 
diff --git a/sysdeps/x86/link_map.h b/sysdeps/x86/link_map.h
index ef1206a9d2..9367ed0889 100644
--- a/sysdeps/x86/link_map.h
+++ b/sysdeps/x86/link_map.h
@@ -19,8 +19,9 @@ 
 /* If this object is enabled with CET.  */
 enum
   {
-    lc_none = 0,			 /* Not enabled with CET.  */
-    lc_ibt = 1 << 0,			 /* Enabled with IBT.  */
-    lc_shstk = 1 << 1,			 /* Enabled with STSHK.  */
+    lc_unknown = 0,			 /* Unknown CET status.  */
+    lc_none = 1 << 0,			 /* Not enabled with CET.  */
+    lc_ibt = 1 << 1,			 /* Enabled with IBT.  */
+    lc_shstk = 1 << 2,			 /* Enabled with STSHK.  */
     lc_ibt_and_shstk = lc_ibt | lc_shstk /* Enabled with both.  */
-  } l_cet:2;
+  } l_cet:3;