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

[Carlos O'Donell <carlos at redhat dot com>]

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?

>  	    {
>  	      unsigned int type = *(unsigned int *) ptr;
>  	      unsigned int datasz = *(unsigned int *) (ptr + 4);
> -- 2.17.1

Cheers,
Carlos.