Re: [PATCH] x86/CET: Don't parse beyond the note end

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Fri, Jul 27, 2018 at 12:22 PM, Carlos O'Donell <carlos@redhat.com> wrote:
> On 07/27/2018 02:22 PM, H.J. Lu wrote:
>> On Fri, Jul 27, 2018 at 11:20 AM, Florian Weimer <fweimer@redhat.com> wrote:
>>> On 07/27/2018 07:56 PM, H.J. Lu wrote:
>>>> Yes, I can reproduce it.  Let me take a look.
>>>
>>> Great.  Did you see the patch I posted?
>>>
>> Please this one instead.
>>
>>
>> -- H.J.
>>
>>
>> 0001-x86-CET-Don-t-parse-beyond-the-note-end.patch
>>
>>
>> From 8de773a7f9225bb9e42eae1263719ca506670087 Mon Sep 17 00:00:00 2001
>> From: "H.J. Lu" <hjl.tools@gmail.com>
>> Date: Fri, 27 Jul 2018 11:17:04 -0700
>> Subject: [PATCH] x86/CET: Don't parse beyond the note end
>>
>> Simply check if "ptr < ptr_end" since "ptr" is always incremented by 8.
>>
>>       * sysdeps/x86/dl-prop.h (_dl_process_cet_property_note): Don't
>>       parse beyond the note end.
>
> This is OK for 2.28, limited to x86, and so low impact to other machine
> testing going on.
>
> Reviewed-by: Carlos O'Donell <carlos@redhat.com>
>
>> ---
>>  sysdeps/x86/dl-prop.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/sysdeps/x86/dl-prop.h b/sysdeps/x86/dl-prop.h
>> index d56e20a6dc..35d3f16a23 100644
>> --- a/sysdeps/x86/dl-prop.h
>> +++ b/sysdeps/x86/dl-prop.h
>> @@ -73,7 +73,7 @@ _dl_process_cet_property_note (struct link_map *l,
>>         unsigned char *ptr = (unsigned char *) (note + 1) + 4;
>>         unsigned char *ptr_end = ptr + note->n_descsz;
>>
>> -       while (1)
>> +       while (ptr < ptr_end)
>
> OK. This only happens with a note that is large enough not to be
> considered invalid, but not large enough to contain the data we
> need. So when we read the start of the property array, we are
> already out of bounds.
>
> Is this an empty property array?

It has

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:

It has a property with all bits cleared.  I checked in a linker patch:

https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=56ad703d56ffe5dc55d5e719a6ec41fd6cf9bfbe

to remove such property.  It should be fixed for binutils 2.31.


-- 
H.J.