[00/12] x86/CET: The last 12 patches to enable Intel CET

Message ID 4519de1c-1561-f064-72ed-295a0f6edb10@redhat.com
State Dropped
Headers

Commit Message

Florian Weimer July 27, 2018, 2:58 p.m. UTC
  On 07/27/2018 04:53 PM, Florian Weimer wrote:
> On 07/26/2018 01:16 PM, Florian Weimer wrote:
>> On 07/21/2018 04:20 PM, H.J. Lu wrote:
>>> These are the last 12 patches to enable Intel CET.  Tested by
>>>
>>> 1. build-many-glibcs.py.
>>> 2. With --enable-cet and without --enable-cet for i686, x86-64 and x32
>>> on non-CET x86-64 processors.
>>> 3. With --enable-cet for x86-64 and x32 on CET SDV using the CET kernel
>>> from cet branch at:
>>>
>>> https://github.com/yyu168/linux_cet/tree/cet
>>>
>>> When the shadow stack (SHSTK) is enabled, makecontext needs to allocate
>>> a new shadow stack to go with the new stack allocated by the caller.
>>> setcontext and swapcontext must properly handle the corresponding shadow
>>> stack when the stack is switched.  Add more tests for user context
>>> functions to provide more coverage for the shadow stack support.
>>
>> Building glibc with --enable-cet currently breaks valgrind (even for 
>> current CPUs which do not support CET):
>>
>>    https://bugzilla.redhat.com/show_bug.cgi?id=1608824
>>
>> Mark is working on a valgrind fix.
> 
> This has now been fixed (in valgrind-3.13.0-22.fc29.x86_64), but I get 
> this instead:
> 
> ==22== Conditional jump or move depends on uninitialised value(s)
> ==22==    at 0x10DF43: _dl_process_cet_property_note (dl-prop.h:82)
> ==22==    by 0x10DF43: _dl_process_pt_note (dl-prop.h:137)
> ==22==    by 0x10E713: _dl_map_object_from_fd (dl-load.c:1170)
> ==22==    by 0x11120B: _dl_map_object (dl-load.c:2241)
> ==22==    by 0x10D92B: dl_main (rtld.c:1050)
> ==22==    by 0x121A6E: _dl_sysdep_start (dl-sysdep.c:253)
> ==22==    by 0x10A1D7: _dl_start_final (rtld.c:413)
> ==22==    by 0x10A1D7: _dl_start (rtld.c:520)
> ==22==    by 0x109117: ??? (in …/build-x86_64-redhat-linux/elf/ld.so)
> ==22==    by 0x3: ???
> ==22==    by 0x1FFF000746: ???
> ==22==    by 0x1FFF000750: ???
> ==22==    by 0x1FFF00075F: ???
> ==22==    by 0x1FFF000770: ???
> ==22==  Uninitialised value was created by a stack allocation
> ==22==    at 0x10DE80: _dl_process_pt_note (dl-prop.h:111)
> 
> One potential cause could be that note traversal loop is buggy.
> 
> GDB says:
> 
> (gdb) bt
> #0  0x000000000010df43 in _dl_process_cet_property_note 
> (align=<optimized out>, size=48, note=0x1ffefff930, l=0x134160)
>      at ../sysdeps/x86/dl-prop.h:82
> #1  _dl_process_pt_note (l=l@entry=0x134160, ph=ph@entry=0x1fff000018, 
> fd=fd@entry=3, fbp=fbp@entry=0x1ffefffe10)
>      at ../sysdeps/x86/dl-prop.h:137
> #2  0x000000000010e714 in _dl_map_object_from_fd 
> (name=name@entry=0x1fff000721 "/usr/bin/true", origname=origname@entry=0x0,
>      fd=<optimized out>, fbp=fbp@entry=0x1ffefffe10, realname=<optimized 
> out>, loader=loader@entry=0x0, l_type=<optimized out>,
>      mode=<optimized out>, stack_endp=<optimized out>, nsid=<optimized 
> out>) at dl-load.c:1170
> #3  0x000000000011120c in _dl_map_object (loader=loader@entry=0x0, 
> name=0x1fff000721 "/usr/bin/true", type=type@entry=0,
>      trace_mode=trace_mode@entry=0, mode=mode@entry=536870912, 
> nsid=nsid@entry=0) at dl-load.c:2241
> #4  0x000000000010d92c in dl_main (phdr=<optimized out>, phnum=10, 
> user_entry=0x1fff0003d8, auxv=<optimized out>) at rtld.c:1050
> #5  0x0000000000121a6f in _dl_sysdep_start 
> (start_argptr=start_argptr@entry=0x1fff0004b0, 
> dl_main=dl_main@entry=0x10a610 <dl_main>)
>      at ../elf/dl-sysdep.c:253
> #6  0x000000000010a1d8 in _dl_start_final (arg=0x1fff0004b0) at rtld.c:413
> #7  _dl_start (arg=0x1fff0004b0) at rtld.c:520
> #8  0x0000000000109118 in _start ()
> 
> The problem seems to be this:
> 
>            unsigned int type = *(unsigned int *) ptr;
>            unsigned int datasz = *(unsigned int *) (ptr + 4);
> 
>            ptr += 8;
>            if ((ptr + datasz) > ptr_end)
>          break;
> 
> We load datasz and then check for having encountered the last note.  I'm 
> checking a fix.

The attached patch should fix this.  Only lightly tested.

Thanks,
Florian
  

Patch

Subject: [PATCH] x86: Fix out of bounds access in CET note parser
To: libc-alpha@sourceware.org

2018-07-27  Florian Weimer  <fweimer@redhat.com>

	* sysdeps/x86/dl-prop.h (_dl_process_cet_property_note): Fixed
	out-of-bounds access in note parser.

diff --git a/sysdeps/x86/dl-prop.h b/sysdeps/x86/dl-prop.h
index d56e20a6dc..9ee70ebc01 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_end - ptr >= 8)
 	    {
 	      unsigned int type = *(unsigned int *) ptr;
 	      unsigned int datasz = *(unsigned int *) (ptr + 4);