This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.
Index Nav:	[Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav:	[Date Prev] [Date Next]	[Thread Prev] [Thread Next]
Other format:	[Raw text]
Re: V2 [PATCH 04/12] x86/CET: Extend arch_prctl syscall for CET control

From: Carlos O'Donell <carlos at redhat dot com>
To: "H.J. Lu" <hjl dot tools at gmail dot com>
Cc: libc-alpha at sourceware dot org
Date: Tue, 24 Jul 2018 09:49:15 -0400
Subject: Re: V2 [PATCH 04/12] x86/CET: Extend arch_prctl syscall for CET control
References: <20180721142035.21059-1-hjl.tools@gmail.com> <20180721142035.21059-5-hjl.tools@gmail.com> <6b2d54fa-28bc-434c-d7ae-405218ac00d4@redhat.com> <20180724132411.GA17974@gmail.com>
On 07/24/2018 09:24 AM, H.J. Lu wrote:
> On Mon, Jul 23, 2018 at 11:05:15PM -0400, Carlos O'Donell wrote:
>> On 07/21/2018 10:20 AM, H.J. Lu wrote:
>>>  /* CET features:
>>>     IBT:   GNU_PROPERTY_X86_FEATURE_1_IBT
>>>     SHSTK: GNU_PROPERTY_X86_FEATURE_1_SHSTK
>>>   */
>>>
>>>  /* Return CET features in unsigned long long *addr:
>>>       features: addr[0].
>>>       shadow stack base address: addr[1].
>>>       shadow stack size: addr[2].
>>>   */
>>>  # define ARCH_CET_STATUS		0x3001
>>>  /* Disable CET features in unsigned int features.  */
>>>  # define ARCH_CET_DISABLE		0x3002
>>>  /* Lock all CET features.  */
>>>  # define ARCH_CET_LOCK			0x3003
>>>  /* Allocate a new shadow stack with unsigned long long *addr:
>>>       IN: requested shadow stack size: *addr.
>>>       OUT: allocated shadow stack address: *addr.
>>>   */
>>>  # define ARCH_CET_ALLOC_SHSTK		0x3004
>>>  /* Return legacy region bitmap info in unsigned long long *addr:
>>>      address: addr[0].
>>>      size: addr[1].
>>>   */
>>>  # define ARCH_CET_LEGACY_BITMAP	0x3005
>>>
>>> 	* sysdeps/unix/sysv/linux/Makefile (sysdep_headers): Add
>>> 	bits/prctl.h.
>>> 	* sysdeps/unix/sysv/linux/bits/prctl.h: New file.
>>> 	* sysdeps/unix/sysv/linux/x86/bits/prctl.h: Likewise.
>>> 	* sysdeps/unix/sysv/linux/sys/prctl.h: Include <bits/prctl.h>.
>>> 	* sysdeps/unix/sysv/linux/x86/cpu-features.c: Include
>>> 	<sys/prctl.h>.
>>> 	(get_cet_status): Call arch_prctl with ARCH_CET_STATUS.
>>> 	* sysdeps/unix/sysv/linux/x86/dl-cet.h: Include  <sys/prctl.h>.
>>> 	(dl_cet_allocate_legacy_bitmap): Call arch_prctl with
>>> 	ARCH_CET_LEGACY_BITMAP.
>>> 	(dl_cet_disable_cet): Call arch_prctl with ARCH_CET_DISABLE.
>>> 	(dl_cet_lock_cet): Call arch_prctl with ARCH_CET_LOCK.
>>> 	* sysdeps/x86/libc-start.c: Include <startup.h>.
>>> ---
>>
>> The prctl.h values don't appear in any published kernel.
>>
>> What's the status for these?
> 
> The CET kernel changes have been submitted and under discussion.  We
> believe that this is the final CET kernel interface.

OK, so this can't be a published public header then. Thank you for
reworking this.
 
>>
>> We can't ship them in the public prctl.h header unless they are already
>> in a public kernel or committed and basically ready to go out in a public
>> kernel.
>>
>> You could rework this to only use the values internally and that would
>> be OK, since no user needs these yet, we are the primary CET user for
>> now.
>>
> 
> Like this?
> 
> 
> H.J.
> ---
>  /* CET features:
>     IBT:   GNU_PROPERTY_X86_FEATURE_1_IBT
>     SHSTK: GNU_PROPERTY_X86_FEATURE_1_SHSTK
>   */
> 
>  /* Return CET features in unsigned long long *addr:
>       features: addr[0].
>       shadow stack base address: addr[1].
>       shadow stack size: addr[2].
>   */
>  # define ARCH_CET_STATUS		0x3001
>  /* Disable CET features in unsigned int features.  */
>  # define ARCH_CET_DISABLE		0x3002
>  /* Lock all CET features.  */
>  # define ARCH_CET_LOCK			0x3003
>  /* Allocate a new shadow stack with unsigned long long *addr:
>       IN: requested shadow stack size: *addr.
>       OUT: allocated shadow stack address: *addr.
>   */
>  # define ARCH_CET_ALLOC_SHSTK		0x3004
>  /* Return legacy region bitmap info in unsigned long long *addr:
>      address: addr[0].
>      size: addr[1].
>   */
>  # define ARCH_CET_LEGACY_BITMAP	0x3005
> 
> Note: sysdeps/unix/sysv/linux/x86/bits/prctl-internal.h should be
> renamed to sysdeps/unix/sysv/linux/x86/bits/prctl.h after the CET
> kernel interface has been committed into the public kernel.
> 
> 	* sysdeps/unix/sysv/linux/Makefile (sysdep_headers): Add
> 	bits/prctl.h.
> 	* sysdeps/unix/sysv/linux/bits/prctl.h: New file.
> 	* sysdeps/unix/sysv/linux/x86/bits/prctl-internal.h: Likewise.
> 	* sysdeps/unix/sysv/linux/sys/prctl.h: Include <bits/prctl.h>.
> 	* sysdeps/unix/sysv/linux/x86/cpu-features.c: Include
> 	<sys/prctl.h> and <bits/prctl-internal.h>.
> 	(get_cet_status): Call arch_prctl with ARCH_CET_STATUS.
> 	* sysdeps/unix/sysv/linux/x86/dl-cet.h: Include <sys/prctl.h>
> 	and <bits/prctl-internal.h>.
> 	(dl_cet_allocate_legacy_bitmap): Call arch_prctl with
> 	ARCH_CET_LEGACY_BITMAP.
> 	(dl_cet_disable_cet): Call arch_prctl with ARCH_CET_DISABLE.
> 	(dl_cet_lock_cet): Call arch_prctl with ARCH_CET_LOCK.
> 	* sysdeps/x86/libc-start.c: Include <startup.h>.

OK to commit, with the prctl-internal.h hack (staging for released
kernel), so long as you confirm the installed prctl.h is empty and
doesn't cause any problems with a build-many-glibcs.sh run.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> ---
>  sysdeps/unix/sysv/linux/Makefile              |  3 +-
>  sysdeps/unix/sysv/linux/bits/prctl.h          | 21 ++++++++
>  sysdeps/unix/sysv/linux/sys/prctl.h           |  1 +
>  .../unix/sysv/linux/x86/bits/prctl-internal.h | 52 +++++++++++++++++++
>  sysdeps/unix/sysv/linux/x86/cpu-features.c    | 10 ++++
>  sysdeps/unix/sysv/linux/x86/dl-cet.h          | 32 +++++++++---
>  sysdeps/x86/libc-start.c                      |  3 ++
>  7 files changed, 115 insertions(+), 7 deletions(-)
>  create mode 100644 sysdeps/unix/sysv/linux/bits/prctl.h
>  create mode 100644 sysdeps/unix/sysv/linux/x86/bits/prctl-internal.h
> 
> diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
> index f71cc39c7e..0bcc5287d9 100644
> --- a/sysdeps/unix/sysv/linux/Makefile
> +++ b/sysdeps/unix/sysv/linux/Makefile
> @@ -40,7 +40,8 @@ sysdep_headers += sys/mount.h sys/acct.h sys/sysctl.h \
>  		  bits/signalfd.h bits/timerfd.h bits/epoll.h \
>  		  bits/socket_type.h bits/syscall.h bits/sysctl.h \
>  		  bits/mman-linux.h bits/mman-shared.h bits/ptrace-shared.h \
> -		  bits/siginfo-arch.h bits/siginfo-consts-arch.h
> +		  bits/siginfo-arch.h bits/siginfo-consts-arch.h \
> +		  bits/prctl.h

OK.

>  
>  tests += tst-clone tst-clone2 tst-clone3 tst-fanotify tst-personality \
>  	 tst-quota tst-sync_file_range tst-sysconf-iov_max tst-ttyname \
> diff --git a/sysdeps/unix/sysv/linux/bits/prctl.h b/sysdeps/unix/sysv/linux/bits/prctl.h
> new file mode 100644
> index 0000000000..be96218066
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/bits/prctl.h
> @@ -0,0 +1,21 @@
> +/* Copyright (C) 2018 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#ifndef _BITS_PRCTL_H
> +#define _BITS_PRCTL_H	1
> +
> +#endif  /* bits/prctl.h */

OK.

> diff --git a/sysdeps/unix/sysv/linux/sys/prctl.h b/sysdeps/unix/sysv/linux/sys/prctl.h
> index 683d16748f..6c4f643c75 100644
> --- a/sysdeps/unix/sysv/linux/sys/prctl.h
> +++ b/sysdeps/unix/sysv/linux/sys/prctl.h
> @@ -20,6 +20,7 @@
>  
>  #include <features.h>
>  #include <linux/prctl.h>  /*  The magic values come from here  */
> +#include <bits/prctl.h>
>  
>  __BEGIN_DECLS
>  
> diff --git a/sysdeps/unix/sysv/linux/x86/bits/prctl-internal.h b/sysdeps/unix/sysv/linux/x86/bits/prctl-internal.h
> new file mode 100644
> index 0000000000..08408d5b88
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/x86/bits/prctl-internal.h
> @@ -0,0 +1,52 @@
> +/* FIXME: Rename this file to prctl.h after the CET kernel interface
> +   has been committed into the public kernel.  */

OK. It's a hack, but we know why, and I'm not ashamed of it, we need ways
to stage work pending kernel releases. Adding constants to a header is an
OK thing to do for a released ABI if needed.

> +#undef _BITS_PRCTL_H
> +
> +/* Copyright (C) 2018 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#ifndef _BITS_PRCTL_H
> +#define _BITS_PRCTL_H	1
> +
> +#ifndef ARCH_CET_STATUS
> +/* CET features:
> +   IBT:   GNU_PROPERTY_X86_FEATURE_1_IBT
> +   SHSTK: GNU_PROPERTY_X86_FEATURE_1_SHSTK
> + */
> +/* Return CET features in unsigned long long *addr:
> +     features: addr[0].
> +     shadow stack base address: addr[1].
> +     shadow stack size: addr[2].
> + */
> +# define ARCH_CET_STATUS	0x3001
> +/* Disable CET features in unsigned int features.  */
> +# define ARCH_CET_DISABLE	0x3002
> +/* Lock all CET features.  */
> +# define ARCH_CET_LOCK		0x3003
> +/* Allocate a new shadow stack with unsigned long long *addr:
> +     IN: requested shadow stack size: *addr.
> +     OUT: allocated shadow stack address: *addr.
> + */
> +# define ARCH_CET_ALLOC_SHSTK	0x3004
> +/* Return legacy region bitmap info in unsigned long long *addr:
> +     address: addr[0].
> +     size: addr[1].
> + */
> +# define ARCH_CET_LEGACY_BITMAP	0x3005
> +#endif /* ARCH_CET_STATUS */
> +
> +#endif  /* bits/prctl.h */

OK.

> diff --git a/sysdeps/unix/sysv/linux/x86/cpu-features.c b/sysdeps/unix/sysv/linux/x86/cpu-features.c
> index 7c9df9b794..c042783ae9 100644
> --- a/sysdeps/unix/sysv/linux/x86/cpu-features.c
> +++ b/sysdeps/unix/sysv/linux/x86/cpu-features.c
> @@ -17,9 +17,19 @@
>     <http://www.gnu.org/licenses/>.  */
>  
>  #if CET_ENABLED
> +# include <sys/prctl.h>
> +/* FIXME: Remove this after <bits/prctl-internal.h> has been renamed to
> +   <bits/prctl.h>.  */
> +# include <bits/prctl-internal.h>

OK.

> +
>  static inline int __attribute__ ((always_inline))
>  get_cet_status (void)
>  {
> +  unsigned long long cet_status[3];
> +  INTERNAL_SYSCALL_DECL (err);
> +  if (INTERNAL_SYSCALL (arch_prctl, err, 2, ARCH_CET_STATUS,
> +			cet_status) == 0)
> +    return cet_status[0];

OK.

>    return 0;
>  }
>  
> diff --git a/sysdeps/unix/sysv/linux/x86/dl-cet.h b/sysdeps/unix/sysv/linux/x86/dl-cet.h
> index ae81e2f2ca..edf8cd7394 100644
> --- a/sysdeps/unix/sysv/linux/x86/dl-cet.h
> +++ b/sysdeps/unix/sysv/linux/x86/dl-cet.h
> @@ -15,23 +15,43 @@
>     License along with the GNU C Library; if not, see
>     <http://www.gnu.org/licenses/>.  */
>  
> +#include <sys/prctl.h>
> +/* FIXME: Remove this after <bits/prctl-internal.h> has been renamed to
> +   <bits/prctl.h>.  */
> +#include <bits/prctl-internal.h>
> +

OK.

>  static inline int __attribute__ ((always_inline))
>  dl_cet_allocate_legacy_bitmap (unsigned long *legacy_bitmap)
>  {
> -  /* FIXME: Need syscall support.  */
> -  return -1;
> +  /* Allocate legacy bitmap.  */
> +  INTERNAL_SYSCALL_DECL (err);
> +#ifdef __LP64__
> +  return (int) INTERNAL_SYSCALL (arch_prctl, err, 2,
> +				 ARCH_CET_LEGACY_BITMAP, legacy_bitmap);
> +#else
> +  unsigned long long legacy_bitmap_u64[2];
> +  int res = INTERNAL_SYSCALL (arch_prctl, err, 2,
> +			      ARCH_CET_LEGACY_BITMAP, legacy_bitmap_u64);
> +  if (res == 0)
> +    {
> +      legacy_bitmap[0] = legacy_bitmap_u64[0];
> +      legacy_bitmap[1] = legacy_bitmap_u64[1];
> +    }
> +  return res;
> +#endif

OK.

>  }
>  
>  static inline int __attribute__ ((always_inline))
>  dl_cet_disable_cet (unsigned int cet_feature)
>  {
> -  /* FIXME: Need syscall support.  */
> -  return -1;
> +  INTERNAL_SYSCALL_DECL (err);
> +  return (int) INTERNAL_SYSCALL (arch_prctl, err, 2, ARCH_CET_DISABLE,
> +				 cet_feature);
>  }
>  

OK.

>  static inline int __attribute__ ((always_inline))
>  dl_cet_lock_cet (void)
>  {
> -  /* FIXME: Need syscall support.  */
> -  return -1;
> +  INTERNAL_SYSCALL_DECL (err);
> +  return (int) INTERNAL_SYSCALL (arch_prctl, err, 2, ARCH_CET_LOCK, 0);

OK.

>  }
> diff --git a/sysdeps/x86/libc-start.c b/sysdeps/x86/libc-start.c
> index 43aba9d061..eb5335c154 100644
> --- a/sysdeps/x86/libc-start.c
> +++ b/sysdeps/x86/libc-start.c
> @@ -16,6 +16,9 @@
>     <http://www.gnu.org/licenses/>.  */
>  
>  #ifndef SHARED
> +/* Define I386_USE_SYSENTER to support syscall during startup in static
> +   PIE.  */
> +# include <startup.h>

OK.

>  # include <ldsodefs.h>
>  # include <cpu-features.h>
>  # include <cpu-features.c>
>
References:
- [PATCH 00/12] x86/CET: The last 12 patches to enable Intel CET
  - From: H.J. Lu
- [PATCH 04/12] x86/CET: Extend arch_prctl syscall for CET control
  - From: H.J. Lu
- Re: [PATCH 04/12] x86/CET: Extend arch_prctl syscall for CET control
  - From: Carlos O'Donell
- V2 [PATCH 04/12] x86/CET: Extend arch_prctl syscall for CET control
  - From: H.J. Lu
Index Nav:	[Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav:	[Date Prev] [Date Next]	[Thread Prev] [Thread Next]