Re: V3 [PATCH 03/24] x86: Support IBT and SHSTK in Intel CET [BZ #21598]

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

On Mon, Jul 16, 2018 at 11:33 AM, Carlos O'Donell <carlos@redhat.com> wrote:
> On 07/10/2018 06:09 PM, H.J. Lu wrote:
>> On Sat, Jul 7, 2018 at 1:01 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> On Fri, Jul 06, 2018 at 03:47:27PM -0400, Carlos O'Donell wrote:
>>>> On 06/13/2018 11:31 AM, H.J. Lu wrote:
>>>>> Intel Control-flow Enforcement Technology (CET) instructions:
>>>> Reviewed.
>>>>
>>>> Looking forward to v2.
>>>>
>> Here is V3.  The difference from V2 is that get_cet_status became:
>>
>> static inline int __attribute__ ((always_inline))
>> get_cet_status (void)
>> {
>>   return 0;
>> }
>>
>
> Thank you for the update.

Here is the diff against my last patch.

> With the following changes:
> - Add suggested comment to sysdeps/generic/dl-prop.h.

Done.

> - Make dl_cet_mark_legacy_region return -1 on error.

I changed it to return -EINVAL since there are

  res = dl_cet_mark_legacy_region (l);
  if (res != 0)
    {
      if (program)
_dl_fatal_printf ("%s: failed to mark legacy code region
\n",
  l->l_name);
      else
_dl_signal_error (-res, l->l_name, "dlopen",
                           ^^^^  The return value is passed to _dl_signal_error.

  N_("failed to mark legacy code region"
));
    }


> - Fix typo s/fail/failed/g
>
> This version is OK.
>

I also added:

/* FIXME: Need syscall support.  */

to sysdeps/unix/sysv/linux/x86/dl-cet.h.  They will be replaced by
syscall in a separate patch.


Thanks.

-- 
H.J.
---
diff --git a/sysdeps/generic/dl-prop.h b/sysdeps/generic/dl-prop.h
index e5fe5d23aa..a2b1d38c79 100644
--- a/sysdeps/generic/dl-prop.h
+++ b/sysdeps/generic/dl-prop.h
@@ -19,6 +19,13 @@
 #ifndef _DL_PROP_H
 #define _DL_PROP_H

+/* The following functions are used by the dynamic loader and the
+   dlopen machinery to process PT_NOTE entries in the binary or
+   shared object.  The notes can be used to change the behaviour of
+   the loader, and as such offer a flexible mechanism for hooking in
+   various checks related to ABI tags or implementing "flag day" ABI
+   transitions.  */
+
 static inline void __attribute__ ((always_inline))
 _rtld_main_check (struct link_map *m, const char *program)
 {
diff --git a/sysdeps/unix/sysv/linux/x86/dl-cet.h
b/sysdeps/unix/sysv/linux/x86/dl-cet.h
index 9903bbe41f..ae81e2f2ca 100644
--- a/sysdeps/unix/sysv/linux/x86/dl-cet.h
+++ b/sysdeps/unix/sysv/linux/x86/dl-cet.h
@@ -18,17 +18,20 @@
 static inline int __attribute__ ((always_inline))
 dl_cet_allocate_legacy_bitmap (unsigned long *legacy_bitmap)
 {
+  /* FIXME: Need syscall support.  */
   return -1;
 }

 static inline int __attribute__ ((always_inline))
 dl_cet_disable_cet (unsigned int cet_feature)
 {
+  /* FIXME: Need syscall support.  */
   return -1;
 }

 static inline int __attribute__ ((always_inline))
 dl_cet_lock_cet (void)
 {
+  /* FIXME: Need syscall support.  */
   return -1;
 }
diff --git a/sysdeps/x86/dl-cet.c b/sysdeps/x86/dl-cet.c
index b3c62bb166..b82ba14e75 100644
--- a/sysdeps/x86/dl-cet.c
+++ b/sysdeps/x86/dl-cet.c
@@ -66,7 +66,7 @@ dl_cet_mark_legacy_region (struct link_map *l)
  ElfW(Addr) end = start + len;

  if ((end / 8) > bitmap_size)
-   return -EFAULT;
+   return -EINVAL;

  p = bitmap + (start / BITS_PER_WORD);
  bits_to_set = BITS_PER_WORD - (start % BITS_PER_WORD);
@@ -240,11 +240,11 @@ mprotect_failure:
    if (res != 0)
      {
        if (program)
- _dl_fatal_printf ("%s: fail to mark legacy code region\n",
+ _dl_fatal_printf ("%s: failed to mark legacy code region\n",
    l->l_name);
        else
  _dl_signal_error (-res, l->l_name, "dlopen",
-   N_("fail to mark legacy code region"));
+   N_("failed to mark legacy code region"));
      }
  }