[PATCH v3 0/8] Memory tagging support

[PATCH v3 0/8] Memory tagging support

[Richard Earnshaw]

This is the third iteration of the patch set to enable memory tagging
in glibc's malloc code.  It mainly addresses the following issues raised
during the previous review:

 - Clean up/add some internal API documentation
 - Remove ROUND_UP_ALLOCATION_SIZE and use some conditionalized code
   in checked_request2size instead.
 - Support MALLOC_CHECK_ in conjuction with _MTAG_ENABLE.

The first two issues are addressed in patch 4 of this series, and the
third in patch 5.  I intend to merge patches 3, 4 and 5 into a single
update to the malloc code before the final commit; I've kept them
separate for now to (hopefully) simplify the review.

The patches have all been rebased against master as of 2020/11/20.

I spent quite a bit of time while working on these looking at whether
the code could be refactored in order to reduce the places where
SIZE_SZ was being added (in different multiples) to various pointers.
I eventually concluded that this wasn't significantly improving the
readability of the code, but one change has survived - I've replaced
usage of 2 * SIZE_SZ with CHUNK_HDR_SZ when it is clear that this is
referring to the header block at the start of a chunk.

I've pushed a copy of this patch series to rearnsha/mte-v3.0, since I
understand some people want to try the patch series as a whole.

R.

Richard Earnshaw (8):
  config: Allow memory tagging to be enabled when configuring glibc
  elf: Add a tunable to control use of tagged memory
  malloc: Basic support for memory tagging in the malloc() family
  malloc: Clean up commentary
  malloc: support MALLOC_CHECK_ in conjunction with _MTAG_ENABLE.
  linux: Add compatibility definitions to sys/prctl.h for MTE
  aarch64: Add sysv specific enabling code for memory tagging
  aarch64: Add aarch64-specific files for memory tagging support

 INSTALL                                       |  14 +
 config.h.in                                   |   3 +
 config.make.in                                |   2 +
 configure                                     |  17 +
 configure.ac                                  |  10 +
 elf/dl-tunables.list                          |   9 +
 malloc/arena.c                                |  59 ++-
 malloc/hooks.c                                |  79 ++--
 malloc/malloc.c                               | 336 ++++++++++++++----
 malloc/malloc.h                               |   7 +
 manual/install.texi                           |  13 +
 manual/tunables.texi                          |  31 ++
 sysdeps/aarch64/Makefile                      |   5 +
 sysdeps/aarch64/__mtag_address_get_tag.S      |  31 ++
 sysdeps/aarch64/__mtag_memset_tag.S           |  46 +++
 sysdeps/aarch64/__mtag_new_tag.S              |  38 ++
 sysdeps/aarch64/__mtag_tag_region.S           |  44 +++
 sysdeps/aarch64/libc-mtag.h                   |  57 +++
 sysdeps/generic/libc-mtag.h                   |  52 +++
 sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h  |   1 +
 sysdeps/unix/sysv/linux/aarch64/bits/mman.h   |   7 +
 .../unix/sysv/linux/aarch64/cpu-features.c    |  28 ++
 .../unix/sysv/linux/aarch64/cpu-features.h    |   1 +
 sysdeps/unix/sysv/linux/sys/prctl.h           |  18 +
 24 files changed, 811 insertions(+), 97 deletions(-)
 create mode 100644 sysdeps/aarch64/__mtag_address_get_tag.S
 create mode 100644 sysdeps/aarch64/__mtag_memset_tag.S
 create mode 100644 sysdeps/aarch64/__mtag_new_tag.S
 create mode 100644 sysdeps/aarch64/__mtag_tag_region.S
 create mode 100644 sysdeps/aarch64/libc-mtag.h
 create mode 100644 sysdeps/generic/libc-mtag.h

-- 
2.29.2

[PATCH v3 1/8] config: Allow memory tagging to be enabled when configuring glibc

[Richard Earnshaw]

[-- Attachment #1: Type: text/plain, Size: 44 bytes --]

This is a multi-part message in MIME format.

[-- Attachment #2: Type: text/plain, Size: 508 bytes --]


This patch adds the configuration machinery to allow memory tagging to be
enabled from the command line via the configure option --enable-memory-tagging.

The current default is off, though in time we may change that once the API
is more stable.
---
 INSTALL             | 14 ++++++++++++++
 config.h.in         |  3 +++
 config.make.in      |  2 ++
 configure           | 17 +++++++++++++++++
 configure.ac        | 10 ++++++++++
 manual/install.texi | 13 +++++++++++++
 6 files changed, 59 insertions(+)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: v3-0001-config-Allow-memory-tagging-to-be-enabled-when-co.patch --]
[-- Type: text/x-patch; name="v3-0001-config-Allow-memory-tagging-to-be-enabled-when-co.patch", Size: 5084 bytes --]

diff --git a/INSTALL b/INSTALL
index 2b00f80df5..f2476df6c0 100644
--- a/INSTALL
+++ b/INSTALL
@@ -142,6 +142,20 @@ if 'CFLAGS' is specified it must enable optimization.  For example:
      non-CET processors.  '--enable-cet' has been tested for i686,
      x86_64 and x32 on CET processors.
 
+'--enable-memory-tagging'
+     Enable memory tagging support on architectures that support it.
+     When the GNU C Library is built with this option then the resulting
+     library will be able to control the use of tagged memory when
+     hardware support is present by use of the tunable
+     'glibc.memtag.enable'.  This includes the generation of tagged
+     memory when using the 'malloc' APIs.
+
+     At present only AArch64 platforms with MTE provide this
+     functionality, although the library will still operate (without
+     memory tagging) on older versions of the architecture.
+
+     The default is to disable support for memory tagging.
+
 '--disable-profile'
      Don't build libraries with profiling information.  You may want to
      use this option if you don't plan to do profiling.
diff --git a/config.h.in b/config.h.in
index b823c8e080..2f4c626c19 100644
--- a/config.h.in
+++ b/config.h.in
@@ -160,6 +160,9 @@
 /* Define if __stack_chk_guard canary should be randomized at program startup.  */
 #undef ENABLE_STACKGUARD_RANDOMIZE
 
+/* Define if memory tagging support should be enabled.  */
+#undef _LIBC_MTAG
+
 /* Package description.  */
 #undef PKGVERSION
 
diff --git a/config.make.in b/config.make.in
index 1ac9417245..7ae27564fd 100644
--- a/config.make.in
+++ b/config.make.in
@@ -84,6 +84,8 @@ mach-interface-list = @mach_interface_list@
 
 experimental-malloc = @experimental_malloc@
 
+memory-tagging = @memory_tagging@
+
 nss-crypt = @libc_cv_nss_crypt@
 static-nss-crypt = @libc_cv_static_nss_crypt@
 
diff --git a/configure b/configure
index 4795e721e5..d22654739f 100755
--- a/configure
+++ b/configure
@@ -676,6 +676,7 @@ build_nscd
 libc_cv_static_nss_crypt
 libc_cv_nss_crypt
 build_crypt
+memory_tagging
 experimental_malloc
 enable_werror
 all_warnings
@@ -781,6 +782,7 @@ enable_all_warnings
 enable_werror
 enable_multi_arch
 enable_experimental_malloc
+enable_memory_tagging
 enable_crypt
 enable_nss_crypt
 enable_systemtap
@@ -1450,6 +1452,8 @@ Optional Features:
                           architectures
   --disable-experimental-malloc
                           disable experimental malloc features
+  --enable-memory-tagging enable memory tagging if supported by the
+                          architecture [default=no]
   --disable-crypt         do not build nor install the passphrase hashing
                           library, libcrypt
   --enable-nss-crypt      enable libcrypt to use nss
@@ -3519,6 +3523,19 @@ fi
 
 
 
+# Check whether --enable-memory-tagging was given.
+if test "${enable_memory_tagging+set}" = set; then :
+  enableval=$enable_memory_tagging; memory_tagging=$enableval
+else
+  memory_tagging=no
+fi
+
+if test "$memory_tagging" = yes; then
+  $as_echo "#define _LIBC_MTAG 1" >>confdefs.h
+
+fi
+
+
 # Check whether --enable-crypt was given.
 if test "${enable_crypt+set}" = set; then :
   enableval=$enable_crypt; build_crypt=$enableval
diff --git a/configure.ac b/configure.ac
index 93e68fb696..78af01d05b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -311,6 +311,16 @@ AC_ARG_ENABLE([experimental-malloc],
 	      [experimental_malloc=yes])
 AC_SUBST(experimental_malloc)
 
+AC_ARG_ENABLE([memory-tagging],
+	      AC_HELP_STRING([--enable-memory-tagging],
+			     [enable memory tagging if supported by the architecture @<:@default=no@:>@]),
+	      [memory_tagging=$enableval],
+	      [memory_tagging=no])
+if test "$memory_tagging" = yes; then
+  AC_DEFINE(_LIBC_MTAG)
+fi
+AC_SUBST(memory_tagging)
+
 AC_ARG_ENABLE([crypt],
               AC_HELP_STRING([--disable-crypt],
                              [do not build nor install the passphrase hashing library, libcrypt]),
diff --git a/manual/install.texi b/manual/install.texi
index 2e164476d5..30fb6aaa46 100644
--- a/manual/install.texi
+++ b/manual/install.texi
@@ -171,6 +171,19 @@ NOTE: @option{--enable-cet} has been tested for i686, x86_64 and x32
 on non-CET processors.  @option{--enable-cet} has been tested for
 i686, x86_64 and x32 on CET processors.
 
+@item --enable-memory-tagging
+Enable memory tagging support on architectures that support it.  When
+@theglibc{} is built with this option then the resulting library will
+be able to control the use of tagged memory when hardware support is
+present by use of the tunable @samp{glibc.memtag.enable}.  This includes
+the generation of tagged memory when using the @code{malloc} APIs.
+
+At present only AArch64 platforms with MTE provide this functionality,
+although the library will still operate (without memory tagging) on
+older versions of the architecture.
+
+The default is to disable support for memory tagging.
+
 @item --disable-profile
 Don't build libraries with profiling information.  You may want to use
 this option if you don't plan to do profiling.

Re: [PATCH v3 1/8] config: Allow memory tagging to be enabled when configuring glibc

[Siddhesh Poyarekar]

On 11/23/20 9:12 PM, Richard Earnshaw via Libc-alpha wrote:
> 
> This patch adds the configuration machinery to allow memory tagging to be
> enabled from the command line via the configure option --enable-memory-tagging.
> 
> The current default is off, though in time we may change that once the API
> is more stable.
> ---
>   INSTALL             | 14 ++++++++++++++
>   config.h.in         |  3 +++
>   config.make.in      |  2 ++
>   configure           | 17 +++++++++++++++++
>   configure.ac        | 10 ++++++++++
>   manual/install.texi | 13 +++++++++++++
>   6 files changed, 59 insertions(+)
> 
> diff --git a/INSTALL b/INSTALL
> index 2b00f80df5..f2476df6c0 100644
> --- a/INSTALL
> +++ b/INSTALL
> @@ -142,6 +142,20 @@ if 'CFLAGS' is specified it must enable optimization.  For example:
>       non-CET processors.  '--enable-cet' has been tested for i686,
>       x86_64 and x32 on CET processors.
>  
> +'--enable-memory-tagging'
> +     Enable memory tagging support on architectures that support it.
> +     When the GNU C Library is built with this option then the resulting
> +     library will be able to control the use of tagged memory when
> +     hardware support is present by use of the tunable
> +     'glibc.memtag.enable'.  This includes the generation of tagged
> +     memory when using the 'malloc' APIs.
> +
> +     At present only AArch64 platforms with MTE provide this
> +     functionality, although the library will still operate (without
> +     memory tagging) on older versions of the architecture.
> +
> +     The default is to disable support for memory tagging.
> +
>  '--disable-profile'
>       Don't build libraries with profiling information.  You may want to
>       use this option if you don't plan to do profiling.
> diff --git a/config.h.in b/config.h.in
> index b823c8e080..2f4c626c19 100644
> --- a/config.h.in
> +++ b/config.h.in
> @@ -160,6 +160,9 @@
>  /* Define if __stack_chk_guard canary should be randomized at program startup.  */
>  #undef ENABLE_STACKGUARD_RANDOMIZE
>  
> +/* Define if memory tagging support should be enabled.  */
> +#undef _LIBC_MTAG
> +
>  /* Package description.  */
>  #undef PKGVERSION
>  
> diff --git a/config.make.in b/config.make.in
> index 1ac9417245..7ae27564fd 100644
> --- a/config.make.in
> +++ b/config.make.in
> @@ -84,6 +84,8 @@ mach-interface-list = @mach_interface_list@
>  
>  experimental-malloc = @experimental_malloc@
>  
> +memory-tagging = @memory_tagging@
> +
>  nss-crypt = @libc_cv_nss_crypt@
>  static-nss-crypt = @libc_cv_static_nss_crypt@
>  
> diff --git a/configure b/configure
> index 4795e721e5..d22654739f 100755
> --- a/configure
> +++ b/configure
> @@ -676,6 +676,7 @@ build_nscd
>  libc_cv_static_nss_crypt
>  libc_cv_nss_crypt
>  build_crypt
> +memory_tagging
>  experimental_malloc
>  enable_werror
>  all_warnings
> @@ -781,6 +782,7 @@ enable_all_warnings
>  enable_werror
>  enable_multi_arch
>  enable_experimental_malloc
> +enable_memory_tagging
>  enable_crypt
>  enable_nss_crypt
>  enable_systemtap
> @@ -1450,6 +1452,8 @@ Optional Features:
>                            architectures
>    --disable-experimental-malloc
>                            disable experimental malloc features
> +  --enable-memory-tagging enable memory tagging if supported by the
> +                          architecture [default=no]

I wonder if that phrasing implies that the flag is disabled if memory 
tagging is not supported by the architecture.  The description in 
INSTALL (Enable memory tagging support on architectures that support it) 
seems a bit more precise.

Siddhesh

Re: [PATCH v3 1/8] config: Allow memory tagging to be enabled when configuring glibc

[Richard Earnshaw (lists)]

On 25/11/2020 15:05, Siddhesh Poyarekar wrote:
> On 11/23/20 9:12 PM, Richard Earnshaw via Libc-alpha wrote:
>>
>> This patch adds the configuration machinery to allow memory tagging to be
>> enabled from the command line via the configure option
>> --enable-memory-tagging.
>>
>> The current default is off, though in time we may change that once the
>> API
>> is more stable.
>> ---
>>   INSTALL             | 14 ++++++++++++++
>>   config.h.in         |  3 +++
>>   config.make.in      |  2 ++
>>   configure           | 17 +++++++++++++++++
>>   configure.ac        | 10 ++++++++++
>>   manual/install.texi | 13 +++++++++++++
>>   6 files changed, 59 insertions(+)
>>
>> diff --git a/INSTALL b/INSTALL
>> index 2b00f80df5..f2476df6c0 100644
>> --- a/INSTALL
>> +++ b/INSTALL
>> @@ -142,6 +142,20 @@ if 'CFLAGS' is specified it must enable
>> optimization.  For example:
>>       non-CET processors.  '--enable-cet' has been tested for i686,
>>       x86_64 and x32 on CET processors.
>>  
>> +'--enable-memory-tagging'
>> +     Enable memory tagging support on architectures that support it.
>> +     When the GNU C Library is built with this option then the resulting
>> +     library will be able to control the use of tagged memory when
>> +     hardware support is present by use of the tunable
>> +     'glibc.memtag.enable'.  This includes the generation of tagged
>> +     memory when using the 'malloc' APIs.
>> +
>> +     At present only AArch64 platforms with MTE provide this
>> +     functionality, although the library will still operate (without
>> +     memory tagging) on older versions of the architecture.
>> +
>> +     The default is to disable support for memory tagging.
>> +
>>  '--disable-profile'
>>       Don't build libraries with profiling information.  You may want to
>>       use this option if you don't plan to do profiling.
>> diff --git a/config.h.in b/config.h.in
>> index b823c8e080..2f4c626c19 100644
>> --- a/config.h.in
>> +++ b/config.h.in
>> @@ -160,6 +160,9 @@
>>  /* Define if __stack_chk_guard canary should be randomized at program
>> startup.  */
>>  #undef ENABLE_STACKGUARD_RANDOMIZE
>>  
>> +/* Define if memory tagging support should be enabled.  */
>> +#undef _LIBC_MTAG
>> +
>>  /* Package description.  */
>>  #undef PKGVERSION
>>  
>> diff --git a/config.make.in b/config.make.in
>> index 1ac9417245..7ae27564fd 100644
>> --- a/config.make.in
>> +++ b/config.make.in
>> @@ -84,6 +84,8 @@ mach-interface-list = @mach_interface_list@
>>  
>>  experimental-malloc = @experimental_malloc@
>>  
>> +memory-tagging = @memory_tagging@
>> +
>>  nss-crypt = @libc_cv_nss_crypt@
>>  static-nss-crypt = @libc_cv_static_nss_crypt@
>>  
>> diff --git a/configure b/configure
>> index 4795e721e5..d22654739f 100755
>> --- a/configure
>> +++ b/configure
>> @@ -676,6 +676,7 @@ build_nscd
>>  libc_cv_static_nss_crypt
>>  libc_cv_nss_crypt
>>  build_crypt
>> +memory_tagging
>>  experimental_malloc
>>  enable_werror
>>  all_warnings
>> @@ -781,6 +782,7 @@ enable_all_warnings
>>  enable_werror
>>  enable_multi_arch
>>  enable_experimental_malloc
>> +enable_memory_tagging
>>  enable_crypt
>>  enable_nss_crypt
>>  enable_systemtap
>> @@ -1450,6 +1452,8 @@ Optional Features:
>>                            architectures
>>    --disable-experimental-malloc
>>                            disable experimental malloc features
>> +  --enable-memory-tagging enable memory tagging if supported by the
>> +                          architecture [default=no]
> 
> I wonder if that phrasing implies that the flag is disabled if memory
> tagging is not supported by the architecture.  The description in
> INSTALL (Enable memory tagging support on architectures that support it)
> seems a bit more precise.
> 
> Siddhesh

I'd be happy to change that to "Enable memory tagging on supported
architectures [default=no]".

R.

Re: [PATCH v3 1/8] config: Allow memory tagging to be enabled when configuring glibc

[Siddhesh Poyarekar]

On 11/25/20 8:39 PM, Richard Earnshaw (lists) wrote:
> I'd be happy to change that to "Enable memory tagging on supported
> architectures [default=no]".

That's good, thanks.

Siddhesh

Re: [PATCH v3 1/8] config: Allow memory tagging to be enabled when configuring glibc

[Adhemerval Zanella]

On 25/11/2020 12:05, Siddhesh Poyarekar wrote:
> On 11/23/20 9:12 PM, Richard Earnshaw via Libc-alpha wrote:
>>
>> This patch adds the configuration machinery to allow memory tagging to be
>> enabled from the command line via the configure option --enable-memory-tagging.
>>
>> The current default is off, though in time we may change that once the API
>> is more stable.
>> ---
>>   INSTALL             | 14 ++++++++++++++
>>   config.h.in         |  3 +++
>>   config.make.in      |  2 ++
>>   configure           | 17 +++++++++++++++++
>>   configure.ac        | 10 ++++++++++
>>   manual/install.texi | 13 +++++++++++++
>>   6 files changed, 59 insertions(+)
>>
>> diff --git a/INSTALL b/INSTALL
>> index 2b00f80df5..f2476df6c0 100644
>> --- a/INSTALL
>> +++ b/INSTALL
>> @@ -142,6 +142,20 @@ if 'CFLAGS' is specified it must enable optimization.  For example:
>>       non-CET processors.  '--enable-cet' has been tested for i686,
>>       x86_64 and x32 on CET processors.
>>  
>> +'--enable-memory-tagging'
>> +     Enable memory tagging support on architectures that support it.
>> +     When the GNU C Library is built with this option then the resulting
>> +     library will be able to control the use of tagged memory when
>> +     hardware support is present by use of the tunable
>> +     'glibc.memtag.enable'.  This includes the generation of tagged
>> +     memory when using the 'malloc' APIs.
>> +
>> +     At present only AArch64 platforms with MTE provide this
>> +     functionality, although the library will still operate (without
>> +     memory tagging) on older versions of the architecture.
>> +
>> +     The default is to disable support for memory tagging.
>> +

Which is the downside of enabling it as default for AArch64 instead of
a configure option?  It would always be defined if --disable-tunables
would be set, so one option would to enable iff tunables is already
defined.

>>  '--disable-profile'
>>       Don't build libraries with profiling information.  You may want to
>>       use this option if you don't plan to do profiling.
>> diff --git a/config.h.in b/config.h.in
>> index b823c8e080..2f4c626c19 100644
>> --- a/config.h.in
>> +++ b/config.h.in
>> @@ -160,6 +160,9 @@
>>  /* Define if __stack_chk_guard canary should be randomized at program startup.  */
>>  #undef ENABLE_STACKGUARD_RANDOMIZE
>>  
>> +/* Define if memory tagging support should be enabled.  */
>> +#undef _LIBC_MTAG

The name is not really following the rest of other define, where
they either use HAVE_* or USE_*. Also, I think there is no need
to use an underscore prefix (it is usually used to define macro
guards).

>> +
>>  /* Package description.  */
>>  #undef PKGVERSION
>>  
>> diff --git a/config.make.in b/config.make.in
>> index 1ac9417245..7ae27564fd 100644
>> --- a/config.make.in
>> +++ b/config.make.in
>> @@ -84,6 +84,8 @@ mach-interface-list = @mach_interface_list@
>>  
>>  experimental-malloc = @experimental_malloc@
>>  
>> +memory-tagging = @memory_tagging@
>> +
>>  nss-crypt = @libc_cv_nss_crypt@
>>  static-nss-crypt = @libc_cv_static_nss_crypt@
>>  
>> diff --git a/configure b/configure
>> index 4795e721e5..d22654739f 100755
>> --- a/configure
>> +++ b/configure
>> @@ -676,6 +676,7 @@ build_nscd
>>  libc_cv_static_nss_crypt
>>  libc_cv_nss_crypt
>>  build_crypt
>> +memory_tagging
>>  experimental_malloc
>>  enable_werror
>>  all_warnings
>> @@ -781,6 +782,7 @@ enable_all_warnings
>>  enable_werror
>>  enable_multi_arch
>>  enable_experimental_malloc
>> +enable_memory_tagging
>>  enable_crypt
>>  enable_nss_crypt
>>  enable_systemtap
>> @@ -1450,6 +1452,8 @@ Optional Features:
>>                            architectures
>>    --disable-experimental-malloc
>>                            disable experimental malloc features
>> +  --enable-memory-tagging enable memory tagging if supported by the
>> +                          architecture [default=no]
> 
> I wonder if that phrasing implies that the flag is disabled if memory tagging is not supported by the architecture.  The description in INSTALL (Enable memory tagging support on architectures that support it) seems a bit more precise.
> 
> Siddhesh

Re: [PATCH v3 1/8] config: Allow memory tagging to be enabled when configuring glibc

[Richard Earnshaw (lists)]

On 25/11/2020 15:12, Adhemerval Zanella via Libc-alpha wrote:
> 
> 
> On 25/11/2020 12:05, Siddhesh Poyarekar wrote:
>> On 11/23/20 9:12 PM, Richard Earnshaw via Libc-alpha wrote:
>>>
>>> This patch adds the configuration machinery to allow memory tagging to be
>>> enabled from the command line via the configure option --enable-memory-tagging.
>>>
>>> The current default is off, though in time we may change that once the API
>>> is more stable.
>>> ---
>>>   INSTALL             | 14 ++++++++++++++
>>>   config.h.in         |  3 +++
>>>   config.make.in      |  2 ++
>>>   configure           | 17 +++++++++++++++++
>>>   configure.ac        | 10 ++++++++++
>>>   manual/install.texi | 13 +++++++++++++
>>>   6 files changed, 59 insertions(+)
>>>
>>> diff --git a/INSTALL b/INSTALL
>>> index 2b00f80df5..f2476df6c0 100644
>>> --- a/INSTALL
>>> +++ b/INSTALL
>>> @@ -142,6 +142,20 @@ if 'CFLAGS' is specified it must enable optimization.  For example:
>>>       non-CET processors.  '--enable-cet' has been tested for i686,
>>>       x86_64 and x32 on CET processors.
>>>  
>>> +'--enable-memory-tagging'
>>> +     Enable memory tagging support on architectures that support it.
>>> +     When the GNU C Library is built with this option then the resulting
>>> +     library will be able to control the use of tagged memory when
>>> +     hardware support is present by use of the tunable
>>> +     'glibc.memtag.enable'.  This includes the generation of tagged
>>> +     memory when using the 'malloc' APIs.
>>> +
>>> +     At present only AArch64 platforms with MTE provide this
>>> +     functionality, although the library will still operate (without
>>> +     memory tagging) on older versions of the architecture.
>>> +
>>> +     The default is to disable support for memory tagging.
>>> +
> 
> Which is the downside of enabling it as default for AArch64 instead of
> a configure option?  It would always be defined if --disable-tunables
> would be set, so one option would to enable iff tunables is already
> defined.


The default of off at present is largely due to this being new and I
didn't want to risk (too much) disturbing existing uses.  My hope is
that the option default will change to on at some point in the
not-too-distant future.

> 
>>>  '--disable-profile'
>>>       Don't build libraries with profiling information.  You may want to
>>>       use this option if you don't plan to do profiling.
>>> diff --git a/config.h.in b/config.h.in
>>> index b823c8e080..2f4c626c19 100644
>>> --- a/config.h.in
>>> +++ b/config.h.in
>>> @@ -160,6 +160,9 @@
>>>  /* Define if __stack_chk_guard canary should be randomized at program startup.  */
>>>  #undef ENABLE_STACKGUARD_RANDOMIZE
>>>  
>>> +/* Define if memory tagging support should be enabled.  */
>>> +#undef _LIBC_MTAG
> 
> The name is not really following the rest of other define, where
> they either use HAVE_* or USE_*. Also, I think there is no need
> to use an underscore prefix (it is usually used to define macro
> guards).

It's mainly just sed work to change the name.  Which do you want?  HAVE_
or USE_ ?

> 
>>> +
>>>  /* Package description.  */
>>>  #undef PKGVERSION
>>>  
>>> diff --git a/config.make.in b/config.make.in
>>> index 1ac9417245..7ae27564fd 100644
>>> --- a/config.make.in
>>> +++ b/config.make.in
>>> @@ -84,6 +84,8 @@ mach-interface-list = @mach_interface_list@
>>>  
>>>  experimental-malloc = @experimental_malloc@
>>>  
>>> +memory-tagging = @memory_tagging@
>>> +
>>>  nss-crypt = @libc_cv_nss_crypt@
>>>  static-nss-crypt = @libc_cv_static_nss_crypt@
>>>  
>>> diff --git a/configure b/configure
>>> index 4795e721e5..d22654739f 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -676,6 +676,7 @@ build_nscd
>>>  libc_cv_static_nss_crypt
>>>  libc_cv_nss_crypt
>>>  build_crypt
>>> +memory_tagging
>>>  experimental_malloc
>>>  enable_werror
>>>  all_warnings
>>> @@ -781,6 +782,7 @@ enable_all_warnings
>>>  enable_werror
>>>  enable_multi_arch
>>>  enable_experimental_malloc
>>> +enable_memory_tagging
>>>  enable_crypt
>>>  enable_nss_crypt
>>>  enable_systemtap
>>> @@ -1450,6 +1452,8 @@ Optional Features:
>>>                            architectures
>>>    --disable-experimental-malloc
>>>                            disable experimental malloc features
>>> +  --enable-memory-tagging enable memory tagging if supported by the
>>> +                          architecture [default=no]
>>
>> I wonder if that phrasing implies that the flag is disabled if memory tagging is not supported by the architecture.  The description in INSTALL (Enable memory tagging support on architectures that support it) seems a bit more precise.
>>
>> Siddhesh

R.

Re: [PATCH v3 1/8] config: Allow memory tagging to be enabled when configuring glibc

[Adhemerval Zanella]

On 25/11/2020 13:11, Richard Earnshaw (lists) wrote:
> On 25/11/2020 15:12, Adhemerval Zanella via Libc-alpha wrote:
>>
>>
>> On 25/11/2020 12:05, Siddhesh Poyarekar wrote:
>>> On 11/23/20 9:12 PM, Richard Earnshaw via Libc-alpha wrote:
>>>>
>>>> This patch adds the configuration machinery to allow memory tagging to be
>>>> enabled from the command line via the configure option --enable-memory-tagging.
>>>>
>>>> The current default is off, though in time we may change that once the API
>>>> is more stable.
>>>> ---
>>>>   INSTALL             | 14 ++++++++++++++
>>>>   config.h.in         |  3 +++
>>>>   config.make.in      |  2 ++
>>>>   configure           | 17 +++++++++++++++++
>>>>   configure.ac        | 10 ++++++++++
>>>>   manual/install.texi | 13 +++++++++++++
>>>>   6 files changed, 59 insertions(+)
>>>>
>>>> diff --git a/INSTALL b/INSTALL
>>>> index 2b00f80df5..f2476df6c0 100644
>>>> --- a/INSTALL
>>>> +++ b/INSTALL
>>>> @@ -142,6 +142,20 @@ if 'CFLAGS' is specified it must enable optimization.  For example:
>>>>       non-CET processors.  '--enable-cet' has been tested for i686,
>>>>       x86_64 and x32 on CET processors.
>>>>  
>>>> +'--enable-memory-tagging'
>>>> +     Enable memory tagging support on architectures that support it.
>>>> +     When the GNU C Library is built with this option then the resulting
>>>> +     library will be able to control the use of tagged memory when
>>>> +     hardware support is present by use of the tunable
>>>> +     'glibc.memtag.enable'.  This includes the generation of tagged
>>>> +     memory when using the 'malloc' APIs.
>>>> +
>>>> +     At present only AArch64 platforms with MTE provide this
>>>> +     functionality, although the library will still operate (without
>>>> +     memory tagging) on older versions of the architecture.
>>>> +
>>>> +     The default is to disable support for memory tagging.
>>>> +
>>
>> Which is the downside of enabling it as default for AArch64 instead of
>> a configure option?  It would always be defined if --disable-tunables
>> would be set, so one option would to enable iff tunables is already
>> defined.
> 
> 
> The default of off at present is largely due to this being new and I
> didn't want to risk (too much) disturbing existing uses.  My hope is
> that the option default will change to on at some point in the
> not-too-distant future.

Right, I am asking because I would like avoid adding another build permutation
which would another testing effort.  To fully test this patch, it already 
requires a MTE enable hardware or emulation; I trying to going from 4 
permutations (default glibc/no-MTE hw, MTE glibc/no-MTE hw, default glibc/MTE hw,
MTE glibc/MTE hw) to just 2 (glibc/no-MTE hw, glibc/MTE hw).

> 
>>
>>>>  '--disable-profile'
>>>>       Don't build libraries with profiling information.  You may want to
>>>>       use this option if you don't plan to do profiling.
>>>> diff --git a/config.h.in b/config.h.in
>>>> index b823c8e080..2f4c626c19 100644
>>>> --- a/config.h.in
>>>> +++ b/config.h.in
>>>> @@ -160,6 +160,9 @@
>>>>  /* Define if __stack_chk_guard canary should be randomized at program startup.  */
>>>>  #undef ENABLE_STACKGUARD_RANDOMIZE
>>>>  
>>>> +/* Define if memory tagging support should be enabled.  */
>>>> +#undef _LIBC_MTAG
>>
>> The name is not really following the rest of other define, where
>> they either use HAVE_* or USE_*. Also, I think there is no need
>> to use an underscore prefix (it is usually used to define macro
>> guards).
> 
> It's mainly just sed work to change the name.  Which do you want?  HAVE_
> or USE_ ?

I think USE_ would make more sense here, but I don't have a strong opinion.

[PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[Richard Earnshaw]

[-- Attachment #1: Type: text/plain, Size: 583 bytes --]


Add a new glibc tunable: mtag.enable, bound to the environment variable
_MTAG_ENABLE.  This is a decimal constant in the range 0-255 but used
as a bit-field.

Bit 0 enables use of tagged memory in the malloc family of functions.
Bit 1 enables precise faulting of tag failure on platforms where this
can be controlled.
Other bits are currently unused, but if set will cause memory tag
checking for the current process to be enabled in the kernel.
---
 elf/dl-tunables.list |  9 +++++++++
 manual/tunables.texi | 31 +++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v3-0002-elf-Add-a-tunable-to-control-use-of-tagged-memory.patch --]
[-- Type: text/x-patch; name="v3-0002-elf-Add-a-tunable-to-control-use-of-tagged-memory.patch", Size: 1940 bytes --]

diff --git a/elf/dl-tunables.list b/elf/dl-tunables.list
index e1d8225128..652cadc334 100644
--- a/elf/dl-tunables.list
+++ b/elf/dl-tunables.list
@@ -141,4 +141,13 @@ glibc {
       default: 512
     }
   }
+
+memtag {
+    enable {
+      type: INT_32
+      minval: 0
+      maxval: 255
+      env_alias: _MTAG_ENABLE
+    }
+  }
 }
diff --git a/manual/tunables.texi b/manual/tunables.texi
index d72d7a5ec0..6ab432a73f 100644
--- a/manual/tunables.texi
+++ b/manual/tunables.texi
@@ -36,6 +36,8 @@ their own namespace.
 * POSIX Thread Tunables:: Tunables in the POSIX thread subsystem
 * Hardware Capability Tunables::  Tunables that modify the hardware
 				  capabilities seen by @theglibc{}
+* Memory Tagging Tunables::  Tunables that control the use of hardware
+			     memory tagging
 @end menu
 
 @node Tunable names
@@ -484,3 +486,32 @@ instead.
 
 This tunable is specific to i386 and x86-64.
 @end deftp
+
+@node Memory Tagging Tunables
+@section Memory Tagging Tunables
+@cindex memory tagging tunables
+
+@deftp {Tunable namespace} glibc.memtag
+If the hardware supports memory tagging, these tunables can be used to
+control the way @theglibc{} uses this feature.  Currently, only AArch64
+supports this feature.
+@end deftp
+
+@deftp Tunable glibc.memtag.enable
+This tunable takes a value between 0 and 255 and acts as a bitmask
+that enables various capabilities.
+
+Bit 0 (the least significant bit) causes the malloc subsystem to allocate
+tagged memory, with each allocation being assigned a random tag.
+
+Bit 1 enables precise faulting mode for tag violations on systems that
+support deferred tag violation reporting.  This may cause programs
+to run more slowly.
+
+Other bits are currently reserved.
+
+@Theglibc{} startup code will automatically enable memory tagging
+support in the kernel if this tunable has any non-zero value.
+
+The default value is @samp{0}, which disables all memory tagging.
+@end deftp

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[Siddhesh Poyarekar]

On 11/23/20 9:12 PM, Richard Earnshaw via Libc-alpha wrote:
> 
> Add a new glibc tunable: mtag.enable, bound to the environment variable
> _MTAG_ENABLE.  This is a decimal constant in the range 0-255 but used
> as a bit-field.
> 
> Bit 0 enables use of tagged memory in the malloc family of functions.
> Bit 1 enables precise faulting of tag failure on platforms where this
> can be controlled.
> Other bits are currently unused, but if set will cause memory tag
> checking for the current process to be enabled in the kernel.
> ---
>   elf/dl-tunables.list |  9 +++++++++
>   manual/tunables.texi | 31 +++++++++++++++++++++++++++++++
>   2 files changed, 40 insertions(+)
> 
> diff --git a/elf/dl-tunables.list b/elf/dl-tunables.list
> index e1d8225128..652cadc334 100644
> --- a/elf/dl-tunables.list
> +++ b/elf/dl-tunables.list
> @@ -141,4 +141,13 @@ glibc {
>        default: 512
>      }
>    }
> +
> +memtag {
> +    enable {
> +      type: INT_32
> +      minval: 0
> +      maxval: 255
> +      env_alias: _MTAG_ENABLE
> +    }
> +  }
>  }
> diff --git a/manual/tunables.texi b/manual/tunables.texi
> index d72d7a5ec0..6ab432a73f 100644
> --- a/manual/tunables.texi
> +++ b/manual/tunables.texi
> @@ -36,6 +36,8 @@ their own namespace.
>  * POSIX Thread Tunables:: Tunables in the POSIX thread subsystem
>  * Hardware Capability Tunables::  Tunables that modify the hardware
>  				  capabilities seen by @theglibc{}
> +* Memory Tagging Tunables::  Tunables that control the use of hardware
> +			     memory tagging
>  @end menu
>  
>  @node Tunable names
> @@ -484,3 +486,32 @@ instead.
>  
>  This tunable is specific to i386 and x86-64.
>  @end deftp
> +
> +@node Memory Tagging Tunables
> +@section Memory Tagging Tunables
> +@cindex memory tagging tunables
> +
> +@deftp {Tunable namespace} glibc.memtag
> +If the hardware supports memory tagging, these tunables can be used to
> +control the way @theglibc{} uses this feature.  Currently, only AArch64
> +supports this feature.
> +@end deftp
> +
> +@deftp Tunable glibc.memtag.enable
> +This tunable takes a value between 0 and 255 and acts as a bitmask
> +that enables various capabilities.
> +
> +Bit 0 (the least significant bit) causes the malloc subsystem to allocate
> +tagged memory, with each allocation being assigned a random tag.
> +
> +Bit 1 enables precise faulting mode for tag violations on systems that
> +support deferred tag violation reporting.  This may cause programs
> +to run more slowly.
> +
> +Other bits are currently reserved.
> +
> +@Theglibc{} startup code will automatically enable memory tagging
> +support in the kernel if this tunable has any non-zero value.
> +
> +The default value is @samp{0}, which disables all memory tagging.
> +@end deftp

This is OK.

Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[H.J. Lu]

On Mon, Nov 23, 2020 at 7:44 AM Richard Earnshaw via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
>
> Add a new glibc tunable: mtag.enable, bound to the environment variable
> _MTAG_ENABLE.  This is a decimal constant in the range 0-255 but used

Do we really need LD_MTAG_ENABLE?

> as a bit-field.
>
> Bit 0 enables use of tagged memory in the malloc family of functions.
> Bit 1 enables precise faulting of tag failure on platforms where this
> can be controlled.
> Other bits are currently unused, but if set will cause memory tag
> checking for the current process to be enabled in the kernel.
> ---
>  elf/dl-tunables.list |  9 +++++++++
>  manual/tunables.texi | 31 +++++++++++++++++++++++++++++++
>  2 files changed, 40 insertions(+)
>


-- 
H.J.

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[Siddhesh Poyarekar]

On 11/25/20 10:05 PM, H.J. Lu via Libc-alpha wrote:
> On Mon, Nov 23, 2020 at 7:44 AM Richard Earnshaw via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
>>
>>
>> Add a new glibc tunable: mtag.enable, bound to the environment variable
>> _MTAG_ENABLE.  This is a decimal constant in the range 0-255 but used
> 
> Do we really need LD_MTAG_ENABLE?
> 

Ahh, thanks for pointing that out, I missed that.  The env aliases are 
really just for compatibility and newer tunables should not add them.

Siddhesh

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[Richard Earnshaw]

On 25/11/2020 16:53, Siddhesh Poyarekar wrote:
> On 11/25/20 10:05 PM, H.J. Lu via Libc-alpha wrote:
>> On Mon, Nov 23, 2020 at 7:44 AM Richard Earnshaw via Libc-alpha
>> <libc-alpha@sourceware.org> wrote:
>>>
>>>
>>> Add a new glibc tunable: mtag.enable, bound to the environment variable
>>> _MTAG_ENABLE.  This is a decimal constant in the range 0-255 but used
>>
>> Do we really need LD_MTAG_ENABLE?
>>
> 
> Ahh, thanks for pointing that out, I missed that.  The env aliases are
> really just for compatibility and newer tunables should not add them.
> 
> Siddhesh

Well,
1) We already have MALLOC_CHECK_; this is just similar but with a wider
scope because ...
2) Turning on MTE enables the feature beyond just GLIBC, since it
activates the prctl call in the kernel to permit calls to mmap to
request tagged memory.  So I think this is beyond the level of a real
tunable (though having a tunable does make things easier in the internal
logic of glibc).

R.

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[Siddhesh Poyarekar]

On 11/25/20 10:28 PM, Richard Earnshaw wrote:
> Well,
> 1) We already have MALLOC_CHECK_; this is just similar but with a wider
> scope because ...

glibc.malloc.check has MALLOC_CHECK_ as a compatibility env alias only 
because the latter predates tunables.

Siddhesh

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[Richard Earnshaw]

On 25/11/2020 17:12, Siddhesh Poyarekar wrote:
> On 11/25/20 10:28 PM, Richard Earnshaw wrote:
>> Well,
>> 1) We already have MALLOC_CHECK_; this is just similar but with a wider
>> scope because ...
> 
> glibc.malloc.check has MALLOC_CHECK_ as a compatibility env alias only
> because the latter predates tunables.
> 
> Siddhesh

But that doesn't really address the issue that the scope of this feature
extends beyond just tuning.

R.

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[Siddhesh Poyarekar]

On 11/25/20 10:54 PM, Richard Earnshaw wrote:
> But that doesn't really address the issue that the scope of this feature
> extends beyond just tuning.

I think that's just a naming issue; it's going to be hard to avoid 
having tunables affect whole programs; I'm surprised that it's taken so 
long to discover a scenario where tunables end up modifying whole 
program behaviour rather than just the library.

Maybe it deserves a top namespace for itself, e.g. proc.memtag so that 
it's clearer that it affects the entire process and not just glibc.

Siddhesh

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[H.J. Lu]

On Wed, Nov 25, 2020 at 9:49 AM Siddhesh Poyarekar <siddhesh@gotplt.org> wrote:
>
> On 11/25/20 10:54 PM, Richard Earnshaw wrote:
> > But that doesn't really address the issue that the scope of this feature
> > extends beyond just tuning.
>
> I think that's just a naming issue; it's going to be hard to avoid
> having tunables affect whole programs; I'm surprised that it's taken so
> long to discover a scenario where tunables end up modifying whole
> program behaviour rather than just the library.

glibc.cpu.x86_shstk and glibc.cpu.x86_ibt also affect the whole program.

> Maybe it deserves a top namespace for itself, e.g. proc.memtag so that
> it's clearer that it affects the entire process and not just glibc.
>
> Siddhesh



-- 
H.J.

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[Siddhesh Poyarekar]

On 11/26/20 12:36 AM, H.J. Lu wrote:
>> I think that's just a naming issue; it's going to be hard to avoid
>> having tunables affect whole programs; I'm surprised that it's taken so
>> long to discover a scenario where tunables end up modifying whole
>> program behaviour rather than just the library.
> 
> glibc.cpu.x86_shstk and glibc.cpu.x86_ibt also affect the whole program.

Richard, since we do have precedent with CET, I'd suggest keeping the 
memory tagging tunable in the glibc namespace.  It ought to be 
glibc.mem.tagging instead of glibc.memtag.enable; glibc.mem then becomes 
home for future memory related tunables that are not limited to malloc.

Another note on this, please put the tunable into 
sysdeps/aarch64/dl-tunables.list since this is aarch64-specific at the 
moment.  We can always bring the tunable into elf/dl-tunables.list if or 
when we either implement memory tagging in software or for another 
architecture.

Siddhesh

PS: Conversations around naming are always the longest and perhaps most 
entertaining too.

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[Richard Earnshaw]

On 26/11/2020 00:47, Siddhesh Poyarekar wrote:
> On 11/26/20 12:36 AM, H.J. Lu wrote:
>>> I think that's just a naming issue; it's going to be hard to avoid
>>> having tunables affect whole programs; I'm surprised that it's taken so
>>> long to discover a scenario where tunables end up modifying whole
>>> program behaviour rather than just the library.
>>
>> glibc.cpu.x86_shstk and glibc.cpu.x86_ibt also affect the whole program.
> 
> Richard, since we do have precedent with CET, I'd suggest keeping the
> memory tagging tunable in the glibc namespace.  It ought to be
> glibc.mem.tagging instead of glibc.memtag.enable; glibc.mem then becomes
> home for future memory related tunables that are not limited to malloc.
> 
> Another note on this, please put the tunable into
> sysdeps/aarch64/dl-tunables.list since this is aarch64-specific at the
> moment.  We can always bring the tunable into elf/dl-tunables.list if or
> when we either implement memory tagging in software or for another
> architecture.
> 

Sure, I can do that if you really think it's the right thing (I presume
this has already been done for other tunables on other architectures, so
that there isn't a lot of additional plumbing needed).  But is it?  It
seems odd to me that the generic malloc code would read a tunable that
only existed in a particular sysdep configuration.  There has to exist
some mechanism for the machine independent code to know that the tagging
behaviour is needed.

R.

> Siddhesh
> 
> PS: Conversations around naming are always the longest and perhaps most
> entertaining too.

For the record, I prefer green ;)

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[Siddhesh Poyarekar]

On 11/26/20 7:45 PM, Richard Earnshaw wrote:
> Sure, I can do that if you really think it's the right thing (I presume
> this has already been done for other tunables on other architectures, so

There's sysdeps/aarch64/dl-tunables.list too, so there's no additional 
plumbing needed...

> that there isn't a lot of additional plumbing needed).  But is it?  It
> seems odd to me that the generic malloc code would read a tunable that
> only existed in a particular sysdep configuration.  There has to exist
> some mechanism for the machine independent code to know that the tagging
> behaviour is needed.

... but I see your point.  How about if we look at the patchset as 
follows, which should make it more clearer.  It doesn't really change 
your patchset in any major way (other than fixing failures and review 
comments), it's only to make the story behind it and hence the design 
decisions more deliberate.

The first part of the patchset (1-3) enables infrastructure to enable 
memory tagging in glibc.  At the project level, this involves adding 
tagging calls (can't call them hooks because that name's taken and also 
invoke nightmares for some) in malloc to allow tagging malloc'd objects. 
  The tagging calls are nops in the default case but support could be 
added either at the architecture level or in the form of a software 
implementation.

The library could add more tag calls in other parts of the library to 
colour them library-internal (e.g. dynamic linker data, glibc internal 
data) but that's for later.

This basically means that memory tagging becomes a library-wide concept 
and hence the glibc.mem.tagging tunable and configury should be 
implemented project-wide, i.e. the way you've done it with your v3 
patchset with just the tunable naming changed.

The second part (6-8, assuming 4 and 5 get subsumed into 3) of the 
patchset implements aarch64 architecture support for memory tagging. 
This involves enabling tagging for the entire process using prctl at 
startup and tagging malloc'd objects.  It is unavoidable that tunables 
will eventually have processwide impact and not just in the library; 
there's precedent for that in x86 CET.

What do you think?

On a slightly different but related point, you may want to think about 
inheritance of the glibc.mem.tagging tunable when you work on v4, i.e.:

1. Should child processes inherit them?  If you're modeling it along the 
lines of MALLOC_CHECK_ (i.e. diagnostics only) then you'd want to keep 
it as default, i.e. no inheritance.  However if you model it as a 
hardening feature, you may want to set security_level to IGNORE so that 
children inherit tagging and forking doesn't become a way to escape 
tagging protections.

2. Should setxid children inherit enabled memory tagging? Again if 
you're modeling it as a hardening feature, then maybe you want to set 
security_level to NONE so that it is inherited by setxid children.  I 
think it will be the first tunable to cross that boundary if you decide 
to take that route!

Siddhesh

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[Richard Earnshaw]

On 26/11/2020 15:27, Siddhesh Poyarekar wrote:
> On 11/26/20 7:45 PM, Richard Earnshaw wrote:
>> Sure, I can do that if you really think it's the right thing (I presume
>> this has already been done for other tunables on other architectures, so
> 
> There's sysdeps/aarch64/dl-tunables.list too, so there's no additional
> plumbing needed...
> 
>> that there isn't a lot of additional plumbing needed).  But is it?  It
>> seems odd to me that the generic malloc code would read a tunable that
>> only existed in a particular sysdep configuration.  There has to exist
>> some mechanism for the machine independent code to know that the tagging
>> behaviour is needed.
> 
> ... but I see your point.  How about if we look at the patchset as
> follows, which should make it more clearer.  It doesn't really change
> your patchset in any major way (other than fixing failures and review
> comments), it's only to make the story behind it and hence the design
> decisions more deliberate.
> 
> The first part of the patchset (1-3) enables infrastructure to enable
> memory tagging in glibc.  At the project level, this involves adding
> tagging calls (can't call them hooks because that name's taken and also
> invoke nightmares for some) in malloc to allow tagging malloc'd objects.
>  The tagging calls are nops in the default case but support could be
> added either at the architecture level or in the form of a software
> implementation.
> 
> The library could add more tag calls in other parts of the library to
> colour them library-internal (e.g. dynamic linker data, glibc internal
> data) but that's for later.
> 
> This basically means that memory tagging becomes a library-wide concept
> and hence the glibc.mem.tagging tunable and configury should be
> implemented project-wide, i.e. the way you've done it with your v3
> patchset with just the tunable naming changed.
> 
> The second part (6-8, assuming 4 and 5 get subsumed into 3) of the
> patchset implements aarch64 architecture support for memory tagging.
> This involves enabling tagging for the entire process using prctl at
> startup and tagging malloc'd objects.  It is unavoidable that tunables
> will eventually have processwide impact and not just in the library;
> there's precedent for that in x86 CET.
> 
> What do you think?

I think it's exactly the way the patch set was structured, I just wasn't
explicit in saying that :)

> 
> On a slightly different but related point, you may want to think about
> inheritance of the glibc.mem.tagging tunable when you work on v4, i.e.:
> 
> 1. Should child processes inherit them?  If you're modeling it along the
> lines of MALLOC_CHECK_ (i.e. diagnostics only) then you'd want to keep
> it as default, i.e. no inheritance.  However if you model it as a
> hardening feature, you may want to set security_level to IGNORE so that
> children inherit tagging and forking doesn't become a way to escape
> tagging protections.
> 
> 2. Should setxid children inherit enabled memory tagging? Again if
> you're modeling it as a hardening feature, then maybe you want to set
> security_level to NONE so that it is inherited by setxid children.  I
> think it will be the first tunable to cross that boundary if you decide
> to take that route!
> 

A good question.  I'd say at this point it's a bit more of a debugging
feature (at least until things have settled down); but longer term it
may well become a hardening feature as well.  Before we can go down that
route, though we'll need to sort out how to mark binaries that are
genuinely incompatible with MTE.  We already know that python's object
management code violates MTE assumptions, for example; either that will
need to be fixed, or we'll need a route to automatically disable MTE
when running programs like that.

So perhaps for now, we'd want to inherit it through normal fork() type
calls, but perhaps not for setxid at this stage, but we may want to
widen it later.  On the other hand, for a security feature you'd perhaps
want a more robust (harder to turn off) mechanism than just modifying a
user-level environment variable.

R.

> Siddhesh

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[H.J. Lu]

On Thu, Nov 26, 2020 at 7:48 AM Richard Earnshaw
<Richard.Earnshaw@foss.arm.com> wrote:
>
> On 26/11/2020 15:27, Siddhesh Poyarekar wrote:
> > On 11/26/20 7:45 PM, Richard Earnshaw wrote:
> >> Sure, I can do that if you really think it's the right thing (I presume
> >> this has already been done for other tunables on other architectures, so
> >
> > There's sysdeps/aarch64/dl-tunables.list too, so there's no additional
> > plumbing needed...
> >
> >> that there isn't a lot of additional plumbing needed).  But is it?  It
> >> seems odd to me that the generic malloc code would read a tunable that
> >> only existed in a particular sysdep configuration.  There has to exist
> >> some mechanism for the machine independent code to know that the tagging
> >> behaviour is needed.
> >
> > ... but I see your point.  How about if we look at the patchset as
> > follows, which should make it more clearer.  It doesn't really change
> > your patchset in any major way (other than fixing failures and review
> > comments), it's only to make the story behind it and hence the design
> > decisions more deliberate.
> >
> > The first part of the patchset (1-3) enables infrastructure to enable
> > memory tagging in glibc.  At the project level, this involves adding
> > tagging calls (can't call them hooks because that name's taken and also
> > invoke nightmares for some) in malloc to allow tagging malloc'd objects.
> >  The tagging calls are nops in the default case but support could be
> > added either at the architecture level or in the form of a software
> > implementation.
> >
> > The library could add more tag calls in other parts of the library to
> > colour them library-internal (e.g. dynamic linker data, glibc internal
> > data) but that's for later.
> >
> > This basically means that memory tagging becomes a library-wide concept
> > and hence the glibc.mem.tagging tunable and configury should be
> > implemented project-wide, i.e. the way you've done it with your v3
> > patchset with just the tunable naming changed.
> >
> > The second part (6-8, assuming 4 and 5 get subsumed into 3) of the
> > patchset implements aarch64 architecture support for memory tagging.
> > This involves enabling tagging for the entire process using prctl at
> > startup and tagging malloc'd objects.  It is unavoidable that tunables
> > will eventually have processwide impact and not just in the library;
> > there's precedent for that in x86 CET.
> >
> > What do you think?
>
> I think it's exactly the way the patch set was structured, I just wasn't
> explicit in saying that :)
>
> >
> > On a slightly different but related point, you may want to think about
> > inheritance of the glibc.mem.tagging tunable when you work on v4, i.e.:
> >
> > 1. Should child processes inherit them?  If you're modeling it along the
> > lines of MALLOC_CHECK_ (i.e. diagnostics only) then you'd want to keep
> > it as default, i.e. no inheritance.  However if you model it as a
> > hardening feature, you may want to set security_level to IGNORE so that
> > children inherit tagging and forking doesn't become a way to escape
> > tagging protections.
> >
> > 2. Should setxid children inherit enabled memory tagging? Again if
> > you're modeling it as a hardening feature, then maybe you want to set
> > security_level to NONE so that it is inherited by setxid children.  I
> > think it will be the first tunable to cross that boundary if you decide
> > to take that route!
> >
>
> A good question.  I'd say at this point it's a bit more of a debugging
> feature (at least until things have settled down); but longer term it
> may well become a hardening feature as well.  Before we can go down that
> route, though we'll need to sort out how to mark binaries that are
> genuinely incompatible with MTE.  We already know that python's object
> management code violates MTE assumptions, for example; either that will
> need to be fixed, or we'll need a route to automatically disable MTE
> when running programs like that.

I think we need to address binary marking first before adding MTE to
glibc.

> So perhaps for now, we'd want to inherit it through normal fork() type
> calls, but perhaps not for setxid at this stage, but we may want to
> widen it later.  On the other hand, for a security feature you'd perhaps
> want a more robust (harder to turn off) mechanism than just modifying a
> user-level environment variable.
>
> R.
>
> > Siddhesh
>


-- 
H.J.

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[Richard Earnshaw]

On 26/11/2020 15:50, H.J. Lu wrote:
> On Thu, Nov 26, 2020 at 7:48 AM Richard Earnshaw
> <Richard.Earnshaw@foss.arm.com> wrote:
>>
>> On 26/11/2020 15:27, Siddhesh Poyarekar wrote:
>>> On 11/26/20 7:45 PM, Richard Earnshaw wrote:
>>>> Sure, I can do that if you really think it's the right thing (I presume
>>>> this has already been done for other tunables on other architectures, so
>>>
>>> There's sysdeps/aarch64/dl-tunables.list too, so there's no additional
>>> plumbing needed...
>>>
>>>> that there isn't a lot of additional plumbing needed).  But is it?  It
>>>> seems odd to me that the generic malloc code would read a tunable that
>>>> only existed in a particular sysdep configuration.  There has to exist
>>>> some mechanism for the machine independent code to know that the tagging
>>>> behaviour is needed.
>>>
>>> ... but I see your point.  How about if we look at the patchset as
>>> follows, which should make it more clearer.  It doesn't really change
>>> your patchset in any major way (other than fixing failures and review
>>> comments), it's only to make the story behind it and hence the design
>>> decisions more deliberate.
>>>
>>> The first part of the patchset (1-3) enables infrastructure to enable
>>> memory tagging in glibc.  At the project level, this involves adding
>>> tagging calls (can't call them hooks because that name's taken and also
>>> invoke nightmares for some) in malloc to allow tagging malloc'd objects.
>>>  The tagging calls are nops in the default case but support could be
>>> added either at the architecture level or in the form of a software
>>> implementation.
>>>
>>> The library could add more tag calls in other parts of the library to
>>> colour them library-internal (e.g. dynamic linker data, glibc internal
>>> data) but that's for later.
>>>
>>> This basically means that memory tagging becomes a library-wide concept
>>> and hence the glibc.mem.tagging tunable and configury should be
>>> implemented project-wide, i.e. the way you've done it with your v3
>>> patchset with just the tunable naming changed.
>>>
>>> The second part (6-8, assuming 4 and 5 get subsumed into 3) of the
>>> patchset implements aarch64 architecture support for memory tagging.
>>> This involves enabling tagging for the entire process using prctl at
>>> startup and tagging malloc'd objects.  It is unavoidable that tunables
>>> will eventually have processwide impact and not just in the library;
>>> there's precedent for that in x86 CET.
>>>
>>> What do you think?
>>
>> I think it's exactly the way the patch set was structured, I just wasn't
>> explicit in saying that :)
>>
>>>
>>> On a slightly different but related point, you may want to think about
>>> inheritance of the glibc.mem.tagging tunable when you work on v4, i.e.:
>>>
>>> 1. Should child processes inherit them?  If you're modeling it along the
>>> lines of MALLOC_CHECK_ (i.e. diagnostics only) then you'd want to keep
>>> it as default, i.e. no inheritance.  However if you model it as a
>>> hardening feature, you may want to set security_level to IGNORE so that
>>> children inherit tagging and forking doesn't become a way to escape
>>> tagging protections.
>>>
>>> 2. Should setxid children inherit enabled memory tagging? Again if
>>> you're modeling it as a hardening feature, then maybe you want to set
>>> security_level to NONE so that it is inherited by setxid children.  I
>>> think it will be the first tunable to cross that boundary if you decide
>>> to take that route!
>>>
>>
>> A good question.  I'd say at this point it's a bit more of a debugging
>> feature (at least until things have settled down); but longer term it
>> may well become a hardening feature as well.  Before we can go down that
>> route, though we'll need to sort out how to mark binaries that are
>> genuinely incompatible with MTE.  We already know that python's object
>> management code violates MTE assumptions, for example; either that will
>> need to be fixed, or we'll need a route to automatically disable MTE
>> when running programs like that.
> 
> I think we need to address binary marking first before adding MTE to
> glibc.

Sorry, I disagree.  While this is a debugging tool there's no need for
binary marking.

Once we have a clearer understanding of what's needed there, we can work
out how best to do the marking.

R.

> 
>> So perhaps for now, we'd want to inherit it through normal fork() type
>> calls, but perhaps not for setxid at this stage, but we may want to
>> widen it later.  On the other hand, for a security feature you'd perhaps
>> want a more robust (harder to turn off) mechanism than just modifying a
>> user-level environment variable.
>>
>> R.
>>
>>> Siddhesh
>>
> 
> 

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[H.J. Lu]

On Thu, Nov 26, 2020 at 8:28 AM Richard Earnshaw
<Richard.Earnshaw@foss.arm.com> wrote:
>
> On 26/11/2020 15:50, H.J. Lu wrote:
> > On Thu, Nov 26, 2020 at 7:48 AM Richard Earnshaw
> > <Richard.Earnshaw@foss.arm.com> wrote:
> >>
> >> On 26/11/2020 15:27, Siddhesh Poyarekar wrote:
> >>> On 11/26/20 7:45 PM, Richard Earnshaw wrote:
> >>>> Sure, I can do that if you really think it's the right thing (I presume
> >>>> this has already been done for other tunables on other architectures, so
> >>>
> >>> There's sysdeps/aarch64/dl-tunables.list too, so there's no additional
> >>> plumbing needed...
> >>>
> >>>> that there isn't a lot of additional plumbing needed).  But is it?  It
> >>>> seems odd to me that the generic malloc code would read a tunable that
> >>>> only existed in a particular sysdep configuration.  There has to exist
> >>>> some mechanism for the machine independent code to know that the tagging
> >>>> behaviour is needed.
> >>>
> >>> ... but I see your point.  How about if we look at the patchset as
> >>> follows, which should make it more clearer.  It doesn't really change
> >>> your patchset in any major way (other than fixing failures and review
> >>> comments), it's only to make the story behind it and hence the design
> >>> decisions more deliberate.
> >>>
> >>> The first part of the patchset (1-3) enables infrastructure to enable
> >>> memory tagging in glibc.  At the project level, this involves adding
> >>> tagging calls (can't call them hooks because that name's taken and also
> >>> invoke nightmares for some) in malloc to allow tagging malloc'd objects.
> >>>  The tagging calls are nops in the default case but support could be
> >>> added either at the architecture level or in the form of a software
> >>> implementation.
> >>>
> >>> The library could add more tag calls in other parts of the library to
> >>> colour them library-internal (e.g. dynamic linker data, glibc internal
> >>> data) but that's for later.
> >>>
> >>> This basically means that memory tagging becomes a library-wide concept
> >>> and hence the glibc.mem.tagging tunable and configury should be
> >>> implemented project-wide, i.e. the way you've done it with your v3
> >>> patchset with just the tunable naming changed.
> >>>
> >>> The second part (6-8, assuming 4 and 5 get subsumed into 3) of the
> >>> patchset implements aarch64 architecture support for memory tagging.
> >>> This involves enabling tagging for the entire process using prctl at
> >>> startup and tagging malloc'd objects.  It is unavoidable that tunables
> >>> will eventually have processwide impact and not just in the library;
> >>> there's precedent for that in x86 CET.
> >>>
> >>> What do you think?
> >>
> >> I think it's exactly the way the patch set was structured, I just wasn't
> >> explicit in saying that :)
> >>
> >>>
> >>> On a slightly different but related point, you may want to think about
> >>> inheritance of the glibc.mem.tagging tunable when you work on v4, i.e.:
> >>>
> >>> 1. Should child processes inherit them?  If you're modeling it along the
> >>> lines of MALLOC_CHECK_ (i.e. diagnostics only) then you'd want to keep
> >>> it as default, i.e. no inheritance.  However if you model it as a
> >>> hardening feature, you may want to set security_level to IGNORE so that
> >>> children inherit tagging and forking doesn't become a way to escape
> >>> tagging protections.
> >>>
> >>> 2. Should setxid children inherit enabled memory tagging? Again if
> >>> you're modeling it as a hardening feature, then maybe you want to set
> >>> security_level to NONE so that it is inherited by setxid children.  I
> >>> think it will be the first tunable to cross that boundary if you decide
> >>> to take that route!
> >>>
> >>
> >> A good question.  I'd say at this point it's a bit more of a debugging
> >> feature (at least until things have settled down); but longer term it
> >> may well become a hardening feature as well.  Before we can go down that
> >> route, though we'll need to sort out how to mark binaries that are
> >> genuinely incompatible with MTE.  We already know that python's object
> >> management code violates MTE assumptions, for example; either that will
> >> need to be fixed, or we'll need a route to automatically disable MTE
> >> when running programs like that.
> >
> > I think we need to address binary marking first before adding MTE to
> > glibc.
>
> Sorry, I disagree.  While this is a debugging tool there's no need for
> binary marking.
>
> Once we have a clearer understanding of what's needed there, we can work
> out how best to do the marking.
>

If the only goal is to have a malloc with memory tag, you should enable
memory tag in an out-of-tree malloc implementation.  It should be sufficient.

-- 
H.J.

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[Richard Earnshaw]

On 26/11/2020 16:51, H.J. Lu wrote:
> On Thu, Nov 26, 2020 at 8:28 AM Richard Earnshaw
> <Richard.Earnshaw@foss.arm.com> wrote:
>>
>> On 26/11/2020 15:50, H.J. Lu wrote:
>>> On Thu, Nov 26, 2020 at 7:48 AM Richard Earnshaw
>>> <Richard.Earnshaw@foss.arm.com> wrote:
>>>>
>>>> On 26/11/2020 15:27, Siddhesh Poyarekar wrote:
>>>>> On 11/26/20 7:45 PM, Richard Earnshaw wrote:
>>>>>> Sure, I can do that if you really think it's the right thing (I presume
>>>>>> this has already been done for other tunables on other architectures, so
>>>>>
>>>>> There's sysdeps/aarch64/dl-tunables.list too, so there's no additional
>>>>> plumbing needed...
>>>>>
>>>>>> that there isn't a lot of additional plumbing needed).  But is it?  It
>>>>>> seems odd to me that the generic malloc code would read a tunable that
>>>>>> only existed in a particular sysdep configuration.  There has to exist
>>>>>> some mechanism for the machine independent code to know that the tagging
>>>>>> behaviour is needed.
>>>>>
>>>>> ... but I see your point.  How about if we look at the patchset as
>>>>> follows, which should make it more clearer.  It doesn't really change
>>>>> your patchset in any major way (other than fixing failures and review
>>>>> comments), it's only to make the story behind it and hence the design
>>>>> decisions more deliberate.
>>>>>
>>>>> The first part of the patchset (1-3) enables infrastructure to enable
>>>>> memory tagging in glibc.  At the project level, this involves adding
>>>>> tagging calls (can't call them hooks because that name's taken and also
>>>>> invoke nightmares for some) in malloc to allow tagging malloc'd objects.
>>>>>  The tagging calls are nops in the default case but support could be
>>>>> added either at the architecture level or in the form of a software
>>>>> implementation.
>>>>>
>>>>> The library could add more tag calls in other parts of the library to
>>>>> colour them library-internal (e.g. dynamic linker data, glibc internal
>>>>> data) but that's for later.
>>>>>
>>>>> This basically means that memory tagging becomes a library-wide concept
>>>>> and hence the glibc.mem.tagging tunable and configury should be
>>>>> implemented project-wide, i.e. the way you've done it with your v3
>>>>> patchset with just the tunable naming changed.
>>>>>
>>>>> The second part (6-8, assuming 4 and 5 get subsumed into 3) of the
>>>>> patchset implements aarch64 architecture support for memory tagging.
>>>>> This involves enabling tagging for the entire process using prctl at
>>>>> startup and tagging malloc'd objects.  It is unavoidable that tunables
>>>>> will eventually have processwide impact and not just in the library;
>>>>> there's precedent for that in x86 CET.
>>>>>
>>>>> What do you think?
>>>>
>>>> I think it's exactly the way the patch set was structured, I just wasn't
>>>> explicit in saying that :)
>>>>
>>>>>
>>>>> On a slightly different but related point, you may want to think about
>>>>> inheritance of the glibc.mem.tagging tunable when you work on v4, i.e.:
>>>>>
>>>>> 1. Should child processes inherit them?  If you're modeling it along the
>>>>> lines of MALLOC_CHECK_ (i.e. diagnostics only) then you'd want to keep
>>>>> it as default, i.e. no inheritance.  However if you model it as a
>>>>> hardening feature, you may want to set security_level to IGNORE so that
>>>>> children inherit tagging and forking doesn't become a way to escape
>>>>> tagging protections.
>>>>>
>>>>> 2. Should setxid children inherit enabled memory tagging? Again if
>>>>> you're modeling it as a hardening feature, then maybe you want to set
>>>>> security_level to NONE so that it is inherited by setxid children.  I
>>>>> think it will be the first tunable to cross that boundary if you decide
>>>>> to take that route!
>>>>>
>>>>
>>>> A good question.  I'd say at this point it's a bit more of a debugging
>>>> feature (at least until things have settled down); but longer term it
>>>> may well become a hardening feature as well.  Before we can go down that
>>>> route, though we'll need to sort out how to mark binaries that are
>>>> genuinely incompatible with MTE.  We already know that python's object
>>>> management code violates MTE assumptions, for example; either that will
>>>> need to be fixed, or we'll need a route to automatically disable MTE
>>>> when running programs like that.
>>>
>>> I think we need to address binary marking first before adding MTE to
>>> glibc.
>>
>> Sorry, I disagree.  While this is a debugging tool there's no need for
>> binary marking.
>>
>> Once we have a clearer understanding of what's needed there, we can work
>> out how best to do the marking.
>>
> 
> If the only goal is to have a malloc with memory tag, you should enable
> memory tag in an out-of-tree malloc implementation.  It should be sufficient.
> 

It's not the only goal.  What's more that would require special linking,
rather than just being able to turn on the feature.

Initially, that might be the main usage, but it's not the ultimate goal.

R.

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[H.J. Lu]

On Thu, Nov 26, 2020 at 9:00 AM Richard Earnshaw
<Richard.Earnshaw@foss.arm.com> wrote:
>
> On 26/11/2020 16:51, H.J. Lu wrote:
> > On Thu, Nov 26, 2020 at 8:28 AM Richard Earnshaw
> > <Richard.Earnshaw@foss.arm.com> wrote:
> >>
> >> On 26/11/2020 15:50, H.J. Lu wrote:
> >>> On Thu, Nov 26, 2020 at 7:48 AM Richard Earnshaw
> >>> <Richard.Earnshaw@foss.arm.com> wrote:
> >>>>
> >>>> On 26/11/2020 15:27, Siddhesh Poyarekar wrote:
> >>>>> On 11/26/20 7:45 PM, Richard Earnshaw wrote:
> >>>>>> Sure, I can do that if you really think it's the right thing (I presume
> >>>>>> this has already been done for other tunables on other architectures, so
> >>>>>
> >>>>> There's sysdeps/aarch64/dl-tunables.list too, so there's no additional
> >>>>> plumbing needed...
> >>>>>
> >>>>>> that there isn't a lot of additional plumbing needed).  But is it?  It
> >>>>>> seems odd to me that the generic malloc code would read a tunable that
> >>>>>> only existed in a particular sysdep configuration.  There has to exist
> >>>>>> some mechanism for the machine independent code to know that the tagging
> >>>>>> behaviour is needed.
> >>>>>
> >>>>> ... but I see your point.  How about if we look at the patchset as
> >>>>> follows, which should make it more clearer.  It doesn't really change
> >>>>> your patchset in any major way (other than fixing failures and review
> >>>>> comments), it's only to make the story behind it and hence the design
> >>>>> decisions more deliberate.
> >>>>>
> >>>>> The first part of the patchset (1-3) enables infrastructure to enable
> >>>>> memory tagging in glibc.  At the project level, this involves adding
> >>>>> tagging calls (can't call them hooks because that name's taken and also
> >>>>> invoke nightmares for some) in malloc to allow tagging malloc'd objects.
> >>>>>  The tagging calls are nops in the default case but support could be
> >>>>> added either at the architecture level or in the form of a software
> >>>>> implementation.
> >>>>>
> >>>>> The library could add more tag calls in other parts of the library to
> >>>>> colour them library-internal (e.g. dynamic linker data, glibc internal
> >>>>> data) but that's for later.
> >>>>>
> >>>>> This basically means that memory tagging becomes a library-wide concept
> >>>>> and hence the glibc.mem.tagging tunable and configury should be
> >>>>> implemented project-wide, i.e. the way you've done it with your v3
> >>>>> patchset with just the tunable naming changed.
> >>>>>
> >>>>> The second part (6-8, assuming 4 and 5 get subsumed into 3) of the
> >>>>> patchset implements aarch64 architecture support for memory tagging.
> >>>>> This involves enabling tagging for the entire process using prctl at
> >>>>> startup and tagging malloc'd objects.  It is unavoidable that tunables
> >>>>> will eventually have processwide impact and not just in the library;
> >>>>> there's precedent for that in x86 CET.
> >>>>>
> >>>>> What do you think?
> >>>>
> >>>> I think it's exactly the way the patch set was structured, I just wasn't
> >>>> explicit in saying that :)
> >>>>
> >>>>>
> >>>>> On a slightly different but related point, you may want to think about
> >>>>> inheritance of the glibc.mem.tagging tunable when you work on v4, i.e.:
> >>>>>
> >>>>> 1. Should child processes inherit them?  If you're modeling it along the
> >>>>> lines of MALLOC_CHECK_ (i.e. diagnostics only) then you'd want to keep
> >>>>> it as default, i.e. no inheritance.  However if you model it as a
> >>>>> hardening feature, you may want to set security_level to IGNORE so that
> >>>>> children inherit tagging and forking doesn't become a way to escape
> >>>>> tagging protections.
> >>>>>
> >>>>> 2. Should setxid children inherit enabled memory tagging? Again if
> >>>>> you're modeling it as a hardening feature, then maybe you want to set
> >>>>> security_level to NONE so that it is inherited by setxid children.  I
> >>>>> think it will be the first tunable to cross that boundary if you decide
> >>>>> to take that route!
> >>>>>
> >>>>
> >>>> A good question.  I'd say at this point it's a bit more of a debugging
> >>>> feature (at least until things have settled down); but longer term it
> >>>> may well become a hardening feature as well.  Before we can go down that
> >>>> route, though we'll need to sort out how to mark binaries that are
> >>>> genuinely incompatible with MTE.  We already know that python's object
> >>>> management code violates MTE assumptions, for example; either that will
> >>>> need to be fixed, or we'll need a route to automatically disable MTE
> >>>> when running programs like that.
> >>>
> >>> I think we need to address binary marking first before adding MTE to
> >>> glibc.
> >>
> >> Sorry, I disagree.  While this is a debugging tool there's no need for
> >> binary marking.
> >>
> >> Once we have a clearer understanding of what's needed there, we can work
> >> out how best to do the marking.
> >>
> >
> > If the only goal is to have a malloc with memory tag, you should enable
> > memory tag in an out-of-tree malloc implementation.  It should be sufficient.
> >
>
> It's not the only goal.  What's more that would require special linking,
> rather than just being able to turn on the feature.
>
> Initially, that might be the main usage, but it's not the ultimate goal.
>

What is your ultimate goal? What is your initial goal?

-- 
H.J.

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[Szabolcs Nagy]

The 11/26/2020 08:51, H.J. Lu via Libc-alpha wrote:
> On Thu, Nov 26, 2020 at 8:28 AM Richard Earnshaw
> <Richard.Earnshaw@foss.arm.com> wrote:
> > On 26/11/2020 15:50, H.J. Lu wrote:
> > > I think we need to address binary marking first before adding MTE to
> > > glibc.
> >
> > Sorry, I disagree.  While this is a debugging tool there's no need for
> > binary marking.
> >
> > Once we have a clearer understanding of what's needed there, we can work
> > out how best to do the marking.
> 
> If the only goal is to have a malloc with memory tag, you should enable
> memory tag in an out-of-tree malloc implementation.  It should be sufficient.

out-of-tree malloc implementation cannot reliably
enable mte: it cannot guarantee that it can initialize
mte before the process becomes multi-threaded or that
it can execute code in all threads before allocated
memory is accessed there, so mte checks can be off
for some threads.

so the mte on/off setting has to be in glibc for now
(out-of-tree implementation is possible, but it won't
be reliable in all cases).

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[H.J. Lu]

On Thu, Nov 26, 2020 at 9:21 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
>
> The 11/26/2020 08:51, H.J. Lu via Libc-alpha wrote:
> > On Thu, Nov 26, 2020 at 8:28 AM Richard Earnshaw
> > <Richard.Earnshaw@foss.arm.com> wrote:
> > > On 26/11/2020 15:50, H.J. Lu wrote:
> > > > I think we need to address binary marking first before adding MTE to
> > > > glibc.
> > >
> > > Sorry, I disagree.  While this is a debugging tool there's no need for
> > > binary marking.
> > >
> > > Once we have a clearer understanding of what's needed there, we can work
> > > out how best to do the marking.
> >
> > If the only goal is to have a malloc with memory tag, you should enable
> > memory tag in an out-of-tree malloc implementation.  It should be sufficient.
>
> out-of-tree malloc implementation cannot reliably
> enable mte: it cannot guarantee that it can initialize
> mte before the process becomes multi-threaded or that
> it can execute code in all threads before allocated
> memory is accessed there, so mte checks can be off
> for some threads.
>
> so the mte on/off setting has to be in glibc for now
> (out-of-tree implementation is possible, but it won't
> be reliable in all cases).

I don't disagree with it.   If the new libc will be installed as the
system glibc, it needs to support binary marker even if there
are currently no binaries with marker.

-- 
H.J.

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[Richard Earnshaw]

On 26/11/2020 17:31, H.J. Lu via Libc-alpha wrote:
> On Thu, Nov 26, 2020 at 9:21 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
>>
>> The 11/26/2020 08:51, H.J. Lu via Libc-alpha wrote:
>>> On Thu, Nov 26, 2020 at 8:28 AM Richard Earnshaw
>>> <Richard.Earnshaw@foss.arm.com> wrote:
>>>> On 26/11/2020 15:50, H.J. Lu wrote:
>>>>> I think we need to address binary marking first before adding MTE to
>>>>> glibc.
>>>>
>>>> Sorry, I disagree.  While this is a debugging tool there's no need for
>>>> binary marking.
>>>>
>>>> Once we have a clearer understanding of what's needed there, we can work
>>>> out how best to do the marking.
>>>
>>> If the only goal is to have a malloc with memory tag, you should enable
>>> memory tag in an out-of-tree malloc implementation.  It should be sufficient.
>>
>> out-of-tree malloc implementation cannot reliably
>> enable mte: it cannot guarantee that it can initialize
>> mte before the process becomes multi-threaded or that
>> it can execute code in all threads before allocated
>> memory is accessed there, so mte checks can be off
>> for some threads.
>>
>> so the mte on/off setting has to be in glibc for now
>> (out-of-tree implementation is possible, but it won't
>> be reliable in all cases).
> 
> I don't disagree with it.   If the new libc will be installed as the
> system glibc, it needs to support binary marker even if there
> are currently no binaries with marker.
> 

Trying to define a binary marker before we know exactly how we want the
binary marker to work is just a good way to define the wrong binary marker.

And for the record, I don't think we know how we'd want that to work at
this point, nor do I think it's necessary for the code proposed to work
as intended.

So I think this is trying to put the cart before the horse.

R.

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[H.J. Lu]

On Thu, Nov 26, 2020 at 9:56 AM Richard Earnshaw
<Richard.Earnshaw@foss.arm.com> wrote:
>
> On 26/11/2020 17:31, H.J. Lu via Libc-alpha wrote:
> > On Thu, Nov 26, 2020 at 9:21 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> >>
> >> The 11/26/2020 08:51, H.J. Lu via Libc-alpha wrote:
> >>> On Thu, Nov 26, 2020 at 8:28 AM Richard Earnshaw
> >>> <Richard.Earnshaw@foss.arm.com> wrote:
> >>>> On 26/11/2020 15:50, H.J. Lu wrote:
> >>>>> I think we need to address binary marking first before adding MTE to
> >>>>> glibc.
> >>>>
> >>>> Sorry, I disagree.  While this is a debugging tool there's no need for
> >>>> binary marking.
> >>>>
> >>>> Once we have a clearer understanding of what's needed there, we can work
> >>>> out how best to do the marking.
> >>>
> >>> If the only goal is to have a malloc with memory tag, you should enable
> >>> memory tag in an out-of-tree malloc implementation.  It should be sufficient.
> >>
> >> out-of-tree malloc implementation cannot reliably
> >> enable mte: it cannot guarantee that it can initialize
> >> mte before the process becomes multi-threaded or that
> >> it can execute code in all threads before allocated
> >> memory is accessed there, so mte checks can be off
> >> for some threads.
> >>
> >> so the mte on/off setting has to be in glibc for now
> >> (out-of-tree implementation is possible, but it won't
> >> be reliable in all cases).
> >
> > I don't disagree with it.   If the new libc will be installed as the
> > system glibc, it needs to support binary marker even if there
> > are currently no binaries with marker.
> >
>
> Trying to define a binary marker before we know exactly how we want the
> binary marker to work is just a good way to define the wrong binary marker.
>
> And for the record, I don't think we know how we'd want that to work at
> this point, nor do I think it's necessary for the code proposed to work
> as intended.

I believe this should be addressed first.

> So I think this is trying to put the cart before the horse.
>
> R.



-- 
H.J.

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[Szabolcs Nagy]

The 11/26/2020 09:31, H.J. Lu wrote:
> I don't disagree with it.   If the new libc will be installed as the
> system glibc, it needs to support binary marker even if there
> are currently no binaries with marker.

i don't yet see how things break if the marking
support comes in a later glibc.

for unmarked binaries the env var behaviour is
fixed and backward compatible.

marked binaries will depend on a new glibc, we
currently don't have a way to do an abi bump
on a binary such that old glibc rejects it
(other than using symbol references or new dyn
relc type), so we may end up with silent breakage
if a new marked binary is used with old glibc.
this is not too bad, but if this is a concern
then e.g. we can do an e_flags check in aarch64
so new flags there are always rejected which will
allow us to bump the mte abi.

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[H.J. Lu]

On Thu, Nov 26, 2020 at 10:06 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
>
> The 11/26/2020 09:31, H.J. Lu wrote:
> > I don't disagree with it.   If the new libc will be installed as the
> > system glibc, it needs to support binary marker even if there
> > are currently no binaries with marker.
>
> i don't yet see how things break if the marking
> support comes in a later glibc.
>

We should anticipate such changes.

> for unmarked binaries the env var behaviour is
> fixed and backward compatible.
>
> marked binaries will depend on a new glibc, we
> currently don't have a way to do an abi bump
> on a binary such that old glibc rejects it
> (other than using symbol references or new dyn
> relc type), so we may end up with silent breakage
> if a new marked binary is used with old glibc.
> this is not too bad, but if this is a concern
> then e.g. we can do an e_flags check in aarch64
> so new flags there are always rejected which will
> allow us to bump the mte abi.

CET enabled binaries don't have such issues.  They are
backward and forward compatible.

-- 
H.J.

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[Andreas Schwab]

On Nov 26 2020, Szabolcs Nagy via Libc-alpha wrote:

> marked binaries will depend on a new glibc, we
> currently don't have a way to do an abi bump
> on a binary such that old glibc rejects it

EI_ABIVERSION

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[Szabolcs Nagy]

The 11/26/2020 19:25, Andreas Schwab wrote:
> On Nov 26 2020, Szabolcs Nagy via Libc-alpha wrote:
> 
> > marked binaries will depend on a new glibc, we
> > currently don't have a way to do an abi bump
> > on a binary such that old glibc rejects it
> 
> EI_ABIVERSION

that does not work for the exe unless this bug got fixed:
https://sourceware.org/legacy-ml/libc-alpha/2019-06/msg00731.html

we want to abi bump kernel loaded binaries too.

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[Florian Weimer]

* Szabolcs Nagy via Libc-alpha:

> The 11/26/2020 19:25, Andreas Schwab wrote:
>> On Nov 26 2020, Szabolcs Nagy via Libc-alpha wrote:
>> 
>> > marked binaries will depend on a new glibc, we
>> > currently don't have a way to do an abi bump
>> > on a binary such that old glibc rejects it
>> 
>> EI_ABIVERSION
>
> that does not work for the exe unless this bug got fixed:
> https://sourceware.org/legacy-ml/libc-alpha/2019-06/msg00731.html
>
> we want to abi bump kernel loaded binaries too.

I posted a generic ABI proposal:

  Critical program headers and dynamic tags
  <https://groups.google.com/g/generic-abi/c/vdG_G4l3N-Y>

We can add it to the GNU ABI if it doesn't make the generic ABI.

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[Siddhesh Poyarekar]

On 11/26/20 11:36 PM, Szabolcs Nagy via Libc-alpha wrote:
> The 11/26/2020 09:31, H.J. Lu wrote:
>> I don't disagree with it.   If the new libc will be installed as the
>> system glibc, it needs to support binary marker even if there
>> are currently no binaries with marker.

As long as the marker is defined in a way that doesn't affect ABI, it 
can be backported.

> i don't yet see how things break if the marking
> support comes in a later glibc.
> 
> for unmarked binaries the env var behaviour is
> fixed and backward compatible.
> 
> marked binaries will depend on a new glibc, we
> currently don't have a way to do an abi bump
> on a binary such that old glibc rejects it
> (other than using symbol references or new dyn
> relc type), so we may end up with silent breakage
> if a new marked binary is used with old glibc.

What kind of breakage?  Just that tagging won't work, which is a 
graceful fallback IMO, the same as what CET does.

> this is not too bad, but if this is a concern
> then e.g. we can do an e_flags check in aarch64
> so new flags there are always rejected which will
> allow us to bump the mte abi.

The ABI bump is precisely what you want to avoid if you're looking for 
the binaries to be usable across systems.

Siddhesh

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[Szabolcs Nagy]

The 11/27/2020 08:29, Siddhesh Poyarekar wrote:
> On 11/26/20 11:36 PM, Szabolcs Nagy via Libc-alpha wrote:
> > i don't yet see how things break if the marking
> > support comes in a later glibc.
> > 
> > for unmarked binaries the env var behaviour is
> > fixed and backward compatible.
> > 
> > marked binaries will depend on a new glibc, we
> > currently don't have a way to do an abi bump
> > on a binary such that old glibc rejects it
> > (other than using symbol references or new dyn
> > relc type), so we may end up with silent breakage
> > if a new marked binary is used with old glibc.
> 
> What kind of breakage?  Just that tagging won't work, which is a graceful
> fallback IMO, the same as what CET does.

i think that's an opt-in marking. here we need an opt-out
marking if we want to turn off the effects of the env var.

(it is ok to leave heap tagging off when it should be on,
but the other direction is not backward compatible, which is
why i suggested an ABI bump to reject marked binaries.)

it is also possible to add a new env var together with the
opt-out marking. (old env var keeps ignoring the marking,
new env var handles it. but many env vars can be confusing)

or we can introduce an opt-in marking, but it's not clear
how well the transition works in practice: you have to be
sure binaries are tagging compatible to mark them and after
some long time when enough binaries are marked the marking
becomes useful. the transition with opt-out is clearer i
think (we can mark binaries as we find issues), but if our
aim is "on by default" tagging then opt-in will be needed.

in short this can go in many directions, but if we have
some insurance (e.g. abi bump mechanism) in place then we
can make the decision later.

> > this is not too bad, but if this is a concern
> > then e.g. we can do an e_flags check in aarch64
> > so new flags there are always rejected which will
> > allow us to bump the mte abi.
> 
> The ABI bump is precisely what you want to avoid if you're looking for the
> binaries to be usable across systems.

yes, we want to avoid it, but load time breakage is still
better than runtime breakage.

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[Siddhesh Poyarekar]

On 11/27/20 4:02 PM, Szabolcs Nagy wrote:
> i think that's an opt-in marking. here we need an opt-out
> marking if we want to turn off the effects of the env var.

Right I assumed it to be an opt-in marker and not opt-out.

> or we can introduce an opt-in marking, but it's not clear
> how well the transition works in practice: you have to be
> sure binaries are tagging compatible to mark them and after
> some long time when enough binaries are marked the marking
> becomes useful. the transition with opt-out is clearer i
> think (we can mark binaries as we find issues), but if our
> aim is "on by default" tagging then opt-in will be needed.

The thing with opt-out though is that it's not really opt-out in the 
real sense.  That is, you need to set the tunable to opt in; and then 
the marker is allowed to override it.  If glibc.mem.tagging is enabled 
by default however, then using the marker to override and opt out makes 
sense.

Or maybe I haven't understood your implementation plan correctly and we 
need to talk more about that as HJ suggests, especially since an ABI 
event seems imminent.

> in short this can go in many directions, but if we have
> some insurance (e.g. abi bump mechanism) in place then we
> can make the decision later.

An opt in marker shouldn't need an ABI bump.

Siddhesh

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[Siddhesh Poyarekar]

On 11/26/20 9:18 PM, Richard Earnshaw wrote:
> I think it's exactly the way the patch set was structured, I just wasn't
> explicit in saying that :)

Agreed, it only reinforces the patch structure and I guess it was also 
for me to clear my own understanding of the patch structure.

> A good question.  I'd say at this point it's a bit more of a debugging
> feature (at least until things have settled down); but longer term it
> may well become a hardening feature as well.  Before we can go down that

Fair enough.

> route, though we'll need to sort out how to mark binaries that are
> genuinely incompatible with MTE.  We already know that python's object
> management code violates MTE assumptions, for example; either that will
> need to be fixed, or we'll need a route to automatically disable MTE
> when running programs like that
On 11/26/20 9:20 PM, H.J. Lu wrote:
 > I think we need to address binary marking first before adding MTE to
 > glibc.

Could you elaborate on why binary marking should block glibc support in 
MTE?  Do you see it changing the design considerably?  Other than the 
tunables (that are not guaranteed to be stable across releases) and 
configure flags, I could not spot any irreversible ABI/API impact that 
would cause issues later.

> So perhaps for now, we'd want to inherit it through normal fork() type
> calls, but perhaps not for setxid at this stage, but we may want to
> widen it later.  On the other hand, for a security feature you'd perhaps
> want a more robust (harder to turn off) mechanism than just modifying a
> user-level environment variable.

Agreed.  I had proposed systemwide tunables to address systemwide 
configuration issues some years ago.  Given how long it took me to get 
to writing tunables in the first place, I'd say another year or so for 
systemwide tunables ;)

Siddhesh

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[H.J. Lu]

On Thu, Nov 26, 2020 at 8:04 AM Siddhesh Poyarekar <siddhesh@gotplt.org> wrote:
> On 11/26/20 9:20 PM, H.J. Lu wrote:
>  > I think we need to address binary marking first before adding MTE to
>  > glibc.
>
> Could you elaborate on why binary marking should block glibc support in
> MTE?  Do you see it changing the design considerably?  Other than the
> tunables (that are not guaranteed to be stable across releases) and
> configure flags, I could not spot any irreversible ABI/API impact that
> would cause issues later.
>

The ultimate goal is to have a single glibc binary which runs everywhere.
Glibc needs to handle static executables, dynamic executables as well as
dlopened shared objects.  Initially, no binaries are marked and memory
tag should be disabled by default.  Tunable can be used to enable memory
tag manually at run-time.  We don't know if there are any issues in the current
patches without considering the memory tag marker support.

-- 
H.J.

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[Siddhesh Poyarekar]

On 11/26/20 9:49 PM, H.J. Lu wrote:
> The ultimate goal is to have a single glibc binary which runs everywhere.
> Glibc needs to handle static executables, dynamic executables as well as
> dlopened shared objects.  Initially, no binaries are marked and memory
> tag should be disabled by default.  Tunable can be used to enable memory

The patchset seems to tick all those boxes.

> tag manually at run-time.  We don't know if there are any issues in the current
> patches without considering the memory tag marker support.

Sure, which is why I asked the question so that we can discuss how 
memory tag marker support in the binary would impact this feature. 
Could you please elaborate on that because I still don't see it.  I can 
see how having binaries marked to always run with tagging enabled could 
be a good way to ensure that they're always executed that way, but it's 
something that could get added on top of the current patchset and 
perhaps even backported since it doesn't affect ABI.

Siddhesh

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[H.J. Lu]

On Thu, Nov 26, 2020 at 9:14 AM Siddhesh Poyarekar <siddhesh@gotplt.org> wrote:
>
> On 11/26/20 9:49 PM, H.J. Lu wrote:
> > The ultimate goal is to have a single glibc binary which runs everywhere.
> > Glibc needs to handle static executables, dynamic executables as well as
> > dlopened shared objects.  Initially, no binaries are marked and memory
> > tag should be disabled by default.  Tunable can be used to enable memory
>
> The patchset seems to tick all those boxes.
>
> > tag manually at run-time.  We don't know if there are any issues in the current
> > patches without considering the memory tag marker support.
>
> Sure, which is why I asked the question so that we can discuss how
> memory tag marker support in the binary would impact this feature.
> Could you please elaborate on that because I still don't see it.  I can

The first few questions are

1.  Where should binary markers be checked?
2.  How should binary marker checking work together with tunables?

> see how having binaries marked to always run with tagging enabled could
> be a good way to ensure that they're always executed that way, but it's
> something that could get added on top of the current patchset and
> perhaps even backported since it doesn't affect ABI.
>
> Siddhesh



-- 
H.J.

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[Siddhesh Poyarekar]

On 11/26/20 10:49 PM, H.J. Lu wrote:
> The first few questions are

OK this is a good start:

> 1.  Where should binary markers be checked?

At early startup alongside the cpu features resolution.  We enable 
tagging if the CPU supports MTE and the marker is set.

> 2.  How should binary marker checking work together with tunables?

The presence of a binary marker enables tagging and a tunable should not 
be able to disable it.  The exception would be systemwide tunables[1] 
where administrators could set sweeping policies for their systems, 
including disabling tagging systemwide if needed.

If binary marker is not present, tunables behave the way it is proposed 
in the patchset.

Siddhesh

[1] Vapourware alert!

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[Richard Earnshaw]

On 27/11/2020 02:45, Siddhesh Poyarekar wrote:
> On 11/26/20 10:49 PM, H.J. Lu wrote:
>> The first few questions are
> 
> OK this is a good start:
> 
>> 1.  Where should binary markers be checked?
> 
> At early startup alongside the cpu features resolution.  We enable
> tagging if the CPU supports MTE and the marker is set.

I think that's backwards.  The default should be to assume that a binary
supports tagging and it should only be disabled if the binary explicitly
says that it is incompatible.

Requiring a tag to be set before you enable tagging will
a) Force a complete recompile of all binaries
b) Result in binaries being incorrectly marked as tagging compatible
when they aren't (because they won't really be audited until they start
failing).  They will then have to be rebuilt again without the tag.

Given b) it then seems obvious that the right way to do this is just to
mark binaries that are known not to work; after they've been audited to
make sure that the reason they don't work is legitimate.

> 
>> 2.  How should binary marker checking work together with tunables?
> 
> The presence of a binary marker enables tagging and a tunable should not
> be able to disable it.  The exception would be systemwide tunables[1]
> where administrators could set sweeping policies for their systems,
> including disabling tagging systemwide if needed.
> 
> If binary marker is not present, tunables behave the way it is proposed
> in the patchset.
> 
> Siddhesh
> 
> [1] Vapourware alert!

R.

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[Richard Earnshaw]

On 27/11/2020 10:40, Richard Earnshaw wrote:
> On 27/11/2020 02:45, Siddhesh Poyarekar wrote:
>> On 11/26/20 10:49 PM, H.J. Lu wrote:
>>> The first few questions are
>>
>> OK this is a good start:
>>
>>> 1.  Where should binary markers be checked?
>>
>> At early startup alongside the cpu features resolution.  We enable
>> tagging if the CPU supports MTE and the marker is set.
> 
> I think that's backwards.  The default should be to assume that a binary
> supports tagging and it should only be disabled if the binary explicitly
> says that it is incompatible.
> 
> Requiring a tag to be set before you enable tagging will
> a) Force a complete recompile of all binaries
> b) Result in binaries being incorrectly marked as tagging compatible
> when they aren't (because they won't really be audited until they start
> failing).  They will then have to be rebuilt again without the tag.
> 
> Given b) it then seems obvious that the right way to do this is just to
> mark binaries that are known not to work; after they've been audited to
> make sure that the reason they don't work is legitimate.
> 
>>
>>> 2.  How should binary marker checking work together with tunables?
>>
>> The presence of a binary marker enables tagging and a tunable should not
>> be able to disable it.  The exception would be systemwide tunables[1]
>> where administrators could set sweeping policies for their systems,
>> including disabling tagging systemwide if needed.
>>
>> If binary marker is not present, tunables behave the way it is proposed
>> in the patchset.
>>
>> Siddhesh
>>
>> [1] Vapourware alert!
> 
> R.
> 

Sorry, I meant to add that if this is to be considered a security
feature it should be an active decision to disable it for a specific
binary, not an active decision to enable it.  What's more, the marker
then can be used to quickly find binaries that are tagging unsafe when
targetting things for audit purposes.

R.

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[Siddhesh Poyarekar]

On 11/27/20 4:19 PM, Richard Earnshaw wrote:
> Sorry, I meant to add that if this is to be considered a security
> feature it should be an active decision to disable it for a specific
> binary, not an active decision to enable it.  What's more, the marker
> then can be used to quickly find binaries that are tagging unsafe when
> targetting things for audit purposes.

I am a bit confused with the contradictory statements about this feature 
and patchset.  Perhaps it is because you want to start out with a debug 
feature and then at some point make it mandatory and we're mixing 
conversations about both.

Thanks,
Siddhesh

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[Richard Earnshaw]

On 27/11/2020 11:32, Siddhesh Poyarekar wrote:
> On 11/27/20 4:19 PM, Richard Earnshaw wrote:
>> Sorry, I meant to add that if this is to be considered a security
>> feature it should be an active decision to disable it for a specific
>> binary, not an active decision to enable it.  What's more, the marker
>> then can be used to quickly find binaries that are tagging unsafe when
>> targetting things for audit purposes.
> 
> I am a bit confused with the contradictory statements about this feature
> and patchset.  Perhaps it is because you want to start out with a debug
> feature and then at some point make it mandatory and we're mixing
> conversations about both.
> 
> Thanks,
> Siddhesh

Yes.  But that's because if this remains just a debugging feature, then
tagging is completely pointless.

R.

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[Siddhesh Poyarekar]

On 11/27/20 4:10 PM, Richard Earnshaw wrote:
> I think that's backwards.  The default should be to assume that a binary
> supports tagging and it should only be disabled if the binary explicitly
> says that it is incompatible.

Sorry, I assumed an opt-in marker.  However opt-in marker being absent 
doesn't really say that the binary does not support tagging, it defers 
the decision to the tunable.

> Requiring a tag to be set before you enable tagging will
> a) Force a complete recompile of all binaries

Binaries shouldn't be needed to be rebuilt to enable tagging; the 
tunable can do that.  If you want distro-wide support for tagging, you 
enable the tunable by default and use the environment variable to opt out.

> b) Result in binaries being incorrectly marked as tagging compatible
> when they aren't (because they won't really be audited until they start
> failing).  They will then have to be rebuilt again without the tag.
> 
> Given b) it then seems obvious that the right way to do this is just to
> mark binaries that are known not to work; after they've been audited to
> make sure that the reason they don't work is legitimate.

With tunable enabled by default, an opt-out marker makes sense because 
then you only rebuild and mark binaries that don't work with tagging and 
have them opt out.

 From a distribution perspective, this could be done in two phases:

1. Phase 1 where you ave the tunable which is disabled by default and no 
marker present.  This makes tagging available to those who want to opt-in

2. Phase 2 is where the tunable is now enabled by default and can be 
overridden with an opt-out marker.  Running this opt-out binary on the 
older version is safe (and shouldn't need an ABI bump) since tunables 
are disabled by default.

Does that make sense?  If not then I'd really like to see a more 
detailed description of how you intend to roll this out.

Siddhesh

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[Richard Earnshaw]

On 27/11/2020 11:27, Siddhesh Poyarekar wrote:
> On 11/27/20 4:10 PM, Richard Earnshaw wrote:
>> I think that's backwards.  The default should be to assume that a binary
>> supports tagging and it should only be disabled if the binary explicitly
>> says that it is incompatible.
> 
> Sorry, I assumed an opt-in marker.  However opt-in marker being absent
> doesn't really say that the binary does not support tagging, it defers
> the decision to the tunable.
> 
>> Requiring a tag to be set before you enable tagging will
>> a) Force a complete recompile of all binaries
> 
> Binaries shouldn't be needed to be rebuilt to enable tagging; the
> tunable can do that.  If you want distro-wide support for tagging, you
> enable the tunable by default and use the environment variable to opt out.
> 
>> b) Result in binaries being incorrectly marked as tagging compatible
>> when they aren't (because they won't really be audited until they start
>> failing).  They will then have to be rebuilt again without the tag.
>>
>> Given b) it then seems obvious that the right way to do this is just to
>> mark binaries that are known not to work; after they've been audited to
>> make sure that the reason they don't work is legitimate.
> 
> With tunable enabled by default, an opt-out marker makes sense because
> then you only rebuild and mark binaries that don't work with tagging and
> have them opt out.
> 
> From a distribution perspective, this could be done in two phases:
> 
> 1. Phase 1 where you ave the tunable which is disabled by default and no
> marker present.  This makes tagging available to those who want to opt-in
> 
> 2. Phase 2 is where the tunable is now enabled by default and can be
> overridden with an opt-out marker.  Running this opt-out binary on the
> older version is safe (and shouldn't need an ABI bump) since tunables
> are disabled by default.
> 
> Does that make sense?  If not then I'd really like to see a more
> detailed description of how you intend to roll this out.
> 
> Siddhesh

Yes, you've captured my thoughts and summarised it better that I was
doing myself.  Thank you.

R.

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[H.J. Lu]

On Fri, Nov 27, 2020 at 4:24 AM Richard Earnshaw
<Richard.Earnshaw@foss.arm.com> wrote:
>
> On 27/11/2020 11:27, Siddhesh Poyarekar wrote:
> > On 11/27/20 4:10 PM, Richard Earnshaw wrote:
> >> I think that's backwards.  The default should be to assume that a binary
> >> supports tagging and it should only be disabled if the binary explicitly
> >> says that it is incompatible.
> >
> > Sorry, I assumed an opt-in marker.  However opt-in marker being absent
> > doesn't really say that the binary does not support tagging, it defers
> > the decision to the tunable.
> >
> >> Requiring a tag to be set before you enable tagging will
> >> a) Force a complete recompile of all binaries
> >
> > Binaries shouldn't be needed to be rebuilt to enable tagging; the
> > tunable can do that.  If you want distro-wide support for tagging, you
> > enable the tunable by default and use the environment variable to opt out.
> >
> >> b) Result in binaries being incorrectly marked as tagging compatible
> >> when they aren't (because they won't really be audited until they start
> >> failing).  They will then have to be rebuilt again without the tag.
> >>
> >> Given b) it then seems obvious that the right way to do this is just to
> >> mark binaries that are known not to work; after they've been audited to
> >> make sure that the reason they don't work is legitimate.
> >
> > With tunable enabled by default, an opt-out marker makes sense because
> > then you only rebuild and mark binaries that don't work with tagging and
> > have them opt out.
> >
> > From a distribution perspective, this could be done in two phases:
> >
> > 1. Phase 1 where you ave the tunable which is disabled by default and no
> > marker present.  This makes tagging available to those who want to opt-in
> >
> > 2. Phase 2 is where the tunable is now enabled by default and can be
> > overridden with an opt-out marker.  Running this opt-out binary on the
> > older version is safe (and shouldn't need an ABI bump) since tunables
> > are disabled by default.
> >
> > Does that make sense?  If not then I'd really like to see a more
> > detailed description of how you intend to roll this out.
> >
> > Siddhesh
>
> Yes, you've captured my thoughts and summarised it better that I was
> doing myself.  Thank you.
>

We need a plan for binary marker first.

-- 
H.J.

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[Szabolcs Nagy]

The 11/27/2020 06:54, H.J. Lu via Libc-alpha wrote:
> On Fri, Nov 27, 2020 at 4:24 AM Richard Earnshaw
> <Richard.Earnshaw@foss.arm.com> wrote:
> > On 27/11/2020 11:27, Siddhesh Poyarekar wrote:
> > > From a distribution perspective, this could be done in two phases:
> > >
> > > 1. Phase 1 where you ave the tunable which is disabled by default and no
> > > marker present.  This makes tagging available to those who want to opt-in
> > >
> > > 2. Phase 2 is where the tunable is now enabled by default and can be
> > > overridden with an opt-out marker.  Running this opt-out binary on the
> > > older version is safe (and shouldn't need an ABI bump) since tunables
> > > are disabled by default.
> > >
> > > Does that make sense?  If not then I'd really like to see a more
> > > detailed description of how you intend to roll this out.
> > >
> > > Siddhesh
> >
> > Yes, you've captured my thoughts and summarised it better that I was
> > doing myself.  Thank you.
> >
> 
> We need a plan for binary marker first.

if we enable heap tagging in those two phases then the first
phase has no abi commitment (tunables are not abi stable).

heap tagging will not be very useful in phase 1 but at least
users can start testing it and report back issues. (assuming
they have hardware or emulator.)

i was hoping we can have an abi stable env var ui, but i'm
fine with tunables only since that sounds the safest.
(if we wait for the marking design and binutils etc support
then we have less chance to see deployment problems early.)

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[H.J. Lu]

On Fri, Nov 27, 2020 at 9:02 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
>
> The 11/27/2020 06:54, H.J. Lu via Libc-alpha wrote:
> > On Fri, Nov 27, 2020 at 4:24 AM Richard Earnshaw
> > <Richard.Earnshaw@foss.arm.com> wrote:
> > > On 27/11/2020 11:27, Siddhesh Poyarekar wrote:
> > > > From a distribution perspective, this could be done in two phases:
> > > >
> > > > 1. Phase 1 where you ave the tunable which is disabled by default and no
> > > > marker present.  This makes tagging available to those who want to opt-in
> > > >
> > > > 2. Phase 2 is where the tunable is now enabled by default and can be
> > > > overridden with an opt-out marker.  Running this opt-out binary on the
> > > > older version is safe (and shouldn't need an ABI bump) since tunables
> > > > are disabled by default.
> > > >
> > > > Does that make sense?  If not then I'd really like to see a more
> > > > detailed description of how you intend to roll this out.
> > > >
> > > > Siddhesh
> > >
> > > Yes, you've captured my thoughts and summarised it better that I was
> > > doing myself.  Thank you.
> > >
> >
> > We need a plan for binary marker first.
>
> if we enable heap tagging in those two phases then the first
> phase has no abi commitment (tunables are not abi stable).
>
> heap tagging will not be very useful in phase 1 but at least
> users can start testing it and report back issues. (assuming
> they have hardware or emulator.)
>
> i was hoping we can have an abi stable env var ui, but i'm
> fine with tunables only since that sounds the safest.
> (if we wait for the marking design and binutils etc support
> then we have less chance to see deployment problems early.)

We should try to get it right if it will be installed as the system glibc.

-- 
H.J.

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[H.J. Lu]

On Thu, Nov 26, 2020 at 6:45 PM Siddhesh Poyarekar <siddhesh@gotplt.org> wrote:
>
> On 11/26/20 10:49 PM, H.J. Lu wrote:
> > The first few questions are
>
> OK this is a good start:
>
> > 1.  Where should binary markers be checked?
>
> At early startup alongside the cpu features resolution.  We enable
> tagging if the CPU supports MTE and the marker is set.

We won't know all the issues before we implement it.

> > 2.  How should binary marker checking work together with tunables?
>
> The presence of a binary marker enables tagging and a tunable should not
> be able to disable it.  The exception would be systemwide tunables[1]
> where administrators could set sweeping policies for their systems,
> including disabling tagging systemwide if needed.

The memory tag implementation should be independent of tunables.
Tunables should just turn on and off a few bits in the memory tag
implementation.  Make the memory tag implementation depend on
tunables seems wrong to me.

> If binary marker is not present, tunables behave the way it is proposed
> in the patchset.
>
> Siddhesh
>
> [1] Vapourware alert!



-- 
H.J.

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[Richard Earnshaw]

On 27/11/2020 14:52, H.J. Lu via Libc-alpha wrote:
> On Thu, Nov 26, 2020 at 6:45 PM Siddhesh Poyarekar <siddhesh@gotplt.org> wrote:
>>
>> On 11/26/20 10:49 PM, H.J. Lu wrote:
>>> The first few questions are
>>
>> OK this is a good start:
>>
>>> 1.  Where should binary markers be checked?
>>
>> At early startup alongside the cpu features resolution.  We enable
>> tagging if the CPU supports MTE and the marker is set.
> 
> We won't know all the issues before we implement it.
> 
>>> 2.  How should binary marker checking work together with tunables?
>>
>> The presence of a binary marker enables tagging and a tunable should not
>> be able to disable it.  The exception would be systemwide tunables[1]
>> where administrators could set sweeping policies for their systems,
>> including disabling tagging systemwide if needed.
> 
> The memory tag implementation should be independent of tunables.
> Tunables should just turn on and off a few bits in the memory tag
> implementation.  Make the memory tag implementation depend on
> tunables seems wrong to me.

That shouldn't matter.  The tunables are documented as not being stable
and nothing else is exposed to the user; so if we want to change things
later, there's nothing to stop that.

R.

> 
>> If binary marker is not present, tunables behave the way it is proposed
>> in the patchset.
>>
>> Siddhesh
>>
>> [1] Vapourware alert!
> 
> 
> 

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[H.J. Lu]

On Fri, Nov 27, 2020 at 8:08 AM Richard Earnshaw
<Richard.Earnshaw@foss.arm.com> wrote:
>
> On 27/11/2020 14:52, H.J. Lu via Libc-alpha wrote:
> > On Thu, Nov 26, 2020 at 6:45 PM Siddhesh Poyarekar <siddhesh@gotplt.org> wrote:
> >>
> >> On 11/26/20 10:49 PM, H.J. Lu wrote:
> >>> The first few questions are
> >>
> >> OK this is a good start:
> >>
> >>> 1.  Where should binary markers be checked?
> >>
> >> At early startup alongside the cpu features resolution.  We enable
> >> tagging if the CPU supports MTE and the marker is set.
> >
> > We won't know all the issues before we implement it.
> >
> >>> 2.  How should binary marker checking work together with tunables?
> >>
> >> The presence of a binary marker enables tagging and a tunable should not
> >> be able to disable it.  The exception would be systemwide tunables[1]
> >> where administrators could set sweeping policies for their systems,
> >> including disabling tagging systemwide if needed.
> >
> > The memory tag implementation should be independent of tunables.
> > Tunables should just turn on and off a few bits in the memory tag
> > implementation.  Make the memory tag implementation depend on
> > tunables seems wrong to me.
>
> That shouldn't matter.  The tunables are documented as not being stable
> and nothing else is exposed to the user; so if we want to change things
> later, there's nothing to stop that.
>

This is not a good user experience and makes it harder to backport.

-- 
H.J.

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[Siddhesh Poyarekar]

On 11/28/20 12:07 AM, H.J. Lu wrote:
>>> The memory tag implementation should be independent of tunables.
>>> Tunables should just turn on and off a few bits in the memory tag
>>> implementation.  Make the memory tag implementation depend on
>>> tunables seems wrong to me.

That comment seems to suggest that you'd like to see finer grained 
control over memory tagging in tunables.  Could you elaborate on that? 
If you're suggesting splitting up or adding more tunables, could you 
suggest the splits you'd like to see?

>> That shouldn't matter.  The tunables are documented as not being stable
>> and nothing else is exposed to the user; so if we want to change things
>> later, there's nothing to stop that.
>>
> 
> This is not a good user experience and makes it harder to backport.

I think that's only partly true.  Backporting changes to the *meaning* 
of tunables can be hard to backport, but simply flipping the default 
isn't quite in the same category.

Siddhesh

Re: [PATCH v3 2/8] elf: Add a tunable to control use of tagged memory

[Szabolcs Nagy]

The 11/26/2020 15:48, Richard Earnshaw via Libc-alpha wrote:
> On 26/11/2020 15:27, Siddhesh Poyarekar wrote:
> > On a slightly different but related point, you may want to think about
> > inheritance of the glibc.mem.tagging tunable when you work on v4, i.e.:
> > 
> > 1. Should child processes inherit them?  If you're modeling it along the
> > lines of MALLOC_CHECK_ (i.e. diagnostics only) then you'd want to keep
> > it as default, i.e. no inheritance.  However if you model it as a
> > hardening feature, you may want to set security_level to IGNORE so that
> > children inherit tagging and forking doesn't become a way to escape
> > tagging protections.
> > 
> > 2. Should setxid children inherit enabled memory tagging? Again if
> > you're modeling it as a hardening feature, then maybe you want to set
> > security_level to NONE so that it is inherited by setxid children.  I
> > think it will be the first tunable to cross that boundary if you decide
> > to take that route!
> > 
> 
> A good question.  I'd say at this point it's a bit more of a debugging
> feature (at least until things have settled down); but longer term it
> may well become a hardening feature as well.  Before we can go down that
> route, though we'll need to sort out how to mark binaries that are
> genuinely incompatible with MTE.  We already know that python's object
> management code violates MTE assumptions, for example; either that will
> need to be fixed, or we'll need a route to automatically disable MTE
> when running programs like that.
> 
> So perhaps for now, we'd want to inherit it through normal fork() type
> calls, but perhaps not for setxid at this stage, but we may want to
> widen it later.  On the other hand, for a security feature you'd perhaps
> want a more robust (harder to turn off) mechanism than just modifying a
> user-level environment variable.

fork must inherit it because otherwise existing heap
pointers don't work with free/realloc.

for now i'd ignore the env var for setxid binaries
(though in principle it should be fine: it is supposed
to be a hardenning feature).

[PATCH v3 3/8] malloc: Basic support for memory tagging in the malloc() family

[Richard Earnshaw]

[-- Attachment #1: Type: text/plain, Size: 962 bytes --]


This patch adds the basic support for memory tagging.

Various flavours are supported, particularly being able to turn on
tagged memory at run-time: this allows the same code to be used on
systems where memory tagging support is not present without neededing
a separate build of glibc.  Also, depending on whether the kernel
supports it, the code will use mmap for the default arena if morecore
does not, or cannot support tagged memory (on AArch64 it is not
available).

All the hooks use function pointers to allow this to work without
needing ifuncs.

malloc: allow memory tagging to be controlled from a tunable
---
 malloc/arena.c              |  43 ++++++++-
 malloc/hooks.c              |   6 ++
 malloc/malloc.c             | 186 ++++++++++++++++++++++++++++--------
 malloc/malloc.h             |   7 ++
 sysdeps/generic/libc-mtag.h |  52 ++++++++++
 5 files changed, 252 insertions(+), 42 deletions(-)
 create mode 100644 sysdeps/generic/libc-mtag.h


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v3-0003-malloc-Basic-support-for-memory-tagging-in-the-ma.patch --]
[-- Type: text/x-patch; name="v3-0003-malloc-Basic-support-for-memory-tagging-in-the-ma.patch", Size: 22451 bytes --]

diff --git a/malloc/arena.c b/malloc/arena.c
index 202daf15b0..835fcc0fb3 100644
--- a/malloc/arena.c
+++ b/malloc/arena.c
@@ -274,17 +274,36 @@ next_env_entry (char ***position)
 #endif
 
 
-#ifdef SHARED
+#if defined(SHARED) || defined(_LIBC_MTAG)
 static void *
 __failing_morecore (ptrdiff_t d)
 {
   return (void *) MORECORE_FAILURE;
 }
+#endif
 
+#ifdef SHARED
 extern struct dl_open_hook *_dl_open_hook;
 libc_hidden_proto (_dl_open_hook);
 #endif
 
+#ifdef _LIBC_MTAG
+static void *
+__mtag_tag_new_usable (void *ptr)
+{
+  if (ptr)
+    ptr = __libc_mtag_tag_region (__libc_mtag_new_tag (ptr),
+				  __malloc_usable_size (ptr));
+  return ptr;
+}
+
+static void *
+__mtag_tag_new_memset (void *ptr, int val, size_t size)
+{
+  return __libc_mtag_memset_with_tag (__libc_mtag_new_tag (ptr), val, size);
+}
+#endif
+
 static void
 ptmalloc_init (void)
 {
@@ -293,6 +312,24 @@ ptmalloc_init (void)
 
   __malloc_initialized = 0;
 
+#ifdef _LIBC_MTAG
+  if ((TUNABLE_GET_FULL (glibc, memtag, enable, int32_t, NULL) & 1) != 0)
+    {
+      /* If the environment says that we should be using tagged memory
+	 and that morecore does not support tagged regions, then
+	 disable it.  */
+      if (__MTAG_SBRK_UNTAGGED)
+	__morecore = __failing_morecore;
+
+      __mtag_mmap_flags = __MTAG_MMAP_FLAGS;
+      __tag_new_memset = __mtag_tag_new_memset;
+      __tag_region = __libc_mtag_tag_region;
+      __tag_new_usable = __mtag_tag_new_usable;
+      __tag_at = __libc_mtag_address_get_tag;
+      __mtag_granule_mask = ~(size_t)(__MTAG_GRANULE_SIZE - 1);
+    }
+#endif
+
 #ifdef SHARED
   /* In case this libc copy is in a non-default namespace, never use brk.
      Likewise if dlopened from statically linked program.  */
@@ -512,7 +549,7 @@ new_heap (size_t size, size_t top_pad)
             }
         }
     }
-  if (__mprotect (p2, size, PROT_READ | PROT_WRITE) != 0)
+  if (__mprotect (p2, size, MTAG_MMAP_FLAGS | PROT_READ | PROT_WRITE) != 0)
     {
       __munmap (p2, HEAP_MAX_SIZE);
       return 0;
@@ -542,7 +579,7 @@ grow_heap (heap_info *h, long diff)
     {
       if (__mprotect ((char *) h + h->mprotect_size,
                       (unsigned long) new_size - h->mprotect_size,
-                      PROT_READ | PROT_WRITE) != 0)
+                      MTAG_MMAP_FLAGS | PROT_READ | PROT_WRITE) != 0)
         return -2;
 
       h->mprotect_size = new_size;
diff --git a/malloc/hooks.c b/malloc/hooks.c
index a2b93e5446..52bb3863cd 100644
--- a/malloc/hooks.c
+++ b/malloc/hooks.c
@@ -56,6 +56,12 @@ static int using_malloc_checking;
 void
 __malloc_check_init (void)
 {
+#if HAVE_TUNABLES && defined (_LIBC_MTAG)
+  /* If memory tagging is enabled, there is no need for the boundary
+     checking cookie as well.  */
+  if ((TUNABLE_GET_FULL (glibc, memtag, enable, int32_t, NULL) & 1) != 0)
+    return;
+#endif
   using_malloc_checking = 1;
   __malloc_hook = malloc_check;
   __free_hook = free_check;
diff --git a/malloc/malloc.c b/malloc/malloc.c
index 5b87bdb081..b866e87bc3 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -242,6 +242,9 @@
 /* For DIAG_PUSH/POP_NEEDS_COMMENT et al.  */
 #include <libc-diag.h>
 
+/* For memory tagging.  */
+#include <libc-mtag.h>
+
 #include <malloc/malloc-internal.h>
 
 /* For SINGLE_THREAD_P.  */
@@ -378,6 +381,52 @@ __malloc_assert (const char *assertion, const char *file, unsigned int line,
 void * __default_morecore (ptrdiff_t);
 void *(*__morecore)(ptrdiff_t) = __default_morecore;
 
+#ifdef _LIBC_MTAG
+static void *
+__default_tag_region (void *ptr, size_t size)
+{
+  return ptr;
+}
+
+static void *
+__default_tag_nop (void *ptr)
+{
+  return ptr;
+}
+
+static int __mtag_mmap_flags = 0;
+static size_t __mtag_granule_mask = ~(size_t)0;
+
+static void *(*__tag_new_memset)(void *, int, size_t) = memset;
+static void *(*__tag_region)(void *, size_t) = __default_tag_region;
+static void *(*__tag_new_usable)(void *) = __default_tag_nop;
+static void *(*__tag_at)(void *) = __default_tag_nop;
+
+# define TAG_NEW_MEMSET(ptr, val, size) __tag_new_memset (ptr, val, size)
+# define TAG_REGION(ptr, size) __tag_region (ptr, size)
+# define TAG_NEW_USABLE(ptr) __tag_new_usable (ptr)
+# define TAG_AT(ptr) __tag_at (ptr)
+#else
+# define TAG_NEW_MEMSET(ptr, val, size) memset (ptr, val, size)
+# define TAG_REGION(ptr, size) (ptr)
+# define TAG_NEW_USABLE(ptr) (ptr)
+# define TAG_AT(ptr) (ptr)
+#endif
+
+/* When using tagged memory, we cannot share the end of the user block
+   with the header for the next chunk, so ensure that we allocate
+   blocks that are rounded up to the granule size.  Take care not to
+   overflow from close to MAX_SIZE_T to a small number.  */
+static inline size_t
+ROUND_UP_ALLOCATION_SIZE(size_t bytes)
+{
+#ifdef _LIBC_MTAG
+  return (bytes + ~__mtag_granule_mask) & __mtag_granule_mask;
+#else
+  return bytes;
+#endif
+}
+
 
 #include <string.h>
 
@@ -1187,8 +1236,9 @@ nextchunk-> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
 
 /* conversion from malloc headers to user pointers, and back */
 
-#define chunk2mem(p)   ((void*)((char*)(p) + 2*SIZE_SZ))
-#define mem2chunk(mem) ((mchunkptr)((char*)(mem) - 2*SIZE_SZ))
+#define chunk2mem(p) ((void*)TAG_AT (((char*)(p) + 2*SIZE_SZ)))
+#define chunk2rawmem(p) ((void*)((char*)(p) + 2*SIZE_SZ))
+#define mem2chunk(mem) ((mchunkptr)TAG_AT (((char*)(mem) - 2*SIZE_SZ)))
 
 /* The smallest possible chunk */
 #define MIN_CHUNK_SIZE        (offsetof(struct malloc_chunk, fd_nextsize))
@@ -1222,6 +1272,7 @@ checked_request2size (size_t req, size_t *sz) __nonnull (1)
 {
   if (__glibc_unlikely (req > PTRDIFF_MAX))
     return false;
+  req = ROUND_UP_ALLOCATION_SIZE (req);
   *sz = request2size (req);
   return true;
 }
@@ -1967,7 +2018,7 @@ do_check_chunk (mstate av, mchunkptr p)
       /* chunk is page-aligned */
       assert (((prev_size (p) + sz) & (GLRO (dl_pagesize) - 1)) == 0);
       /* mem is aligned */
-      assert (aligned_OK (chunk2mem (p)));
+      assert (aligned_OK (chunk2rawmem (p)));
     }
 }
 
@@ -1991,7 +2042,7 @@ do_check_free_chunk (mstate av, mchunkptr p)
   if ((unsigned long) (sz) >= MINSIZE)
     {
       assert ((sz & MALLOC_ALIGN_MASK) == 0);
-      assert (aligned_OK (chunk2mem (p)));
+      assert (aligned_OK (chunk2rawmem (p)));
       /* ... matching footer field */
       assert (prev_size (next_chunk (p)) == sz);
       /* ... and is fully consolidated */
@@ -2070,7 +2121,7 @@ do_check_remalloced_chunk (mstate av, mchunkptr p, INTERNAL_SIZE_T s)
   assert ((sz & MALLOC_ALIGN_MASK) == 0);
   assert ((unsigned long) (sz) >= MINSIZE);
   /* ... and alignment */
-  assert (aligned_OK (chunk2mem (p)));
+  assert (aligned_OK (chunk2rawmem (p)));
   /* chunk is less than MINSIZE more than request */
   assert ((long) (sz) - (long) (s) >= 0);
   assert ((long) (sz) - (long) (s + MINSIZE) < 0);
@@ -2325,7 +2376,8 @@ sysmalloc (INTERNAL_SIZE_T nb, mstate av)
       /* Don't try if size wraps around 0 */
       if ((unsigned long) (size) > (unsigned long) (nb))
         {
-          mm = (char *) (MMAP (0, size, PROT_READ | PROT_WRITE, 0));
+          mm = (char *) (MMAP (0, size,
+			       MTAG_MMAP_FLAGS | PROT_READ | PROT_WRITE, 0));
 
           if (mm != MAP_FAILED)
             {
@@ -2339,14 +2391,14 @@ sysmalloc (INTERNAL_SIZE_T nb, mstate av)
 
               if (MALLOC_ALIGNMENT == 2 * SIZE_SZ)
                 {
-                  /* For glibc, chunk2mem increases the address by 2*SIZE_SZ and
+                  /* For glibc, chunk2rawmem increases the address by 2*SIZE_SZ and
                      MALLOC_ALIGN_MASK is 2*SIZE_SZ-1.  Each mmap'ed area is page
                      aligned and therefore definitely MALLOC_ALIGN_MASK-aligned.  */
-                  assert (((INTERNAL_SIZE_T) chunk2mem (mm) & MALLOC_ALIGN_MASK) == 0);
+                  assert (((INTERNAL_SIZE_T) chunk2rawmem (mm) & MALLOC_ALIGN_MASK) == 0);
                   front_misalign = 0;
                 }
               else
-                front_misalign = (INTERNAL_SIZE_T) chunk2mem (mm) & MALLOC_ALIGN_MASK;
+                front_misalign = (INTERNAL_SIZE_T) chunk2rawmem (mm) & MALLOC_ALIGN_MASK;
               if (front_misalign > 0)
                 {
                   correction = MALLOC_ALIGNMENT - front_misalign;
@@ -2518,7 +2570,9 @@ sysmalloc (INTERNAL_SIZE_T nb, mstate av)
           /* Don't try if size wraps around 0 */
           if ((unsigned long) (size) > (unsigned long) (nb))
             {
-              char *mbrk = (char *) (MMAP (0, size, PROT_READ | PROT_WRITE, 0));
+              char *mbrk = (char *) (MMAP (0, size,
+					   MTAG_MMAP_FLAGS | PROT_READ | PROT_WRITE,
+					   0));
 
               if (mbrk != MAP_FAILED)
                 {
@@ -2589,7 +2643,7 @@ sysmalloc (INTERNAL_SIZE_T nb, mstate av)
 
                   /* Guarantee alignment of first new chunk made from this space */
 
-                  front_misalign = (INTERNAL_SIZE_T) chunk2mem (brk) & MALLOC_ALIGN_MASK;
+                  front_misalign = (INTERNAL_SIZE_T) chunk2rawmem (brk) & MALLOC_ALIGN_MASK;
                   if (front_misalign > 0)
                     {
                       /*
@@ -2647,10 +2701,10 @@ sysmalloc (INTERNAL_SIZE_T nb, mstate av)
                 {
                   if (MALLOC_ALIGNMENT == 2 * SIZE_SZ)
                     /* MORECORE/mmap must correctly align */
-                    assert (((unsigned long) chunk2mem (brk) & MALLOC_ALIGN_MASK) == 0);
+                    assert (((unsigned long) chunk2rawmem (brk) & MALLOC_ALIGN_MASK) == 0);
                   else
                     {
-                      front_misalign = (INTERNAL_SIZE_T) chunk2mem (brk) & MALLOC_ALIGN_MASK;
+                      front_misalign = (INTERNAL_SIZE_T) chunk2rawmem (brk) & MALLOC_ALIGN_MASK;
                       if (front_misalign > 0)
                         {
                           /*
@@ -2835,7 +2889,7 @@ munmap_chunk (mchunkptr p)
   if (DUMPED_MAIN_ARENA_CHUNK (p))
     return;
 
-  uintptr_t mem = (uintptr_t) chunk2mem (p);
+  uintptr_t mem = (uintptr_t) chunk2rawmem (p);
   uintptr_t block = (uintptr_t) p - prev_size (p);
   size_t total_size = prev_size (p) + size;
   /* Unfortunately we have to do the compilers job by hand here.  Normally
@@ -2890,7 +2944,7 @@ mremap_chunk (mchunkptr p, size_t new_size)
 
   p = (mchunkptr) (cp + offset);
 
-  assert (aligned_OK (chunk2mem (p)));
+  assert (aligned_OK (chunk2rawmem (p)));
 
   assert (prev_size (p) == offset);
   set_head (p, (new_size - offset) | IS_MMAPPED);
@@ -3071,14 +3125,15 @@ __libc_malloc (size_t bytes)
       && tcache
       && tcache->counts[tc_idx] > 0)
     {
-      return tcache_get (tc_idx);
+      victim = tcache_get (tc_idx);
+      return TAG_NEW_USABLE (victim);
     }
   DIAG_POP_NEEDS_COMMENT;
 #endif
 
   if (SINGLE_THREAD_P)
     {
-      victim = _int_malloc (&main_arena, bytes);
+      victim = TAG_NEW_USABLE (_int_malloc (&main_arena, bytes));
       assert (!victim || chunk_is_mmapped (mem2chunk (victim)) ||
 	      &main_arena == arena_for_chunk (mem2chunk (victim)));
       return victim;
@@ -3099,6 +3154,8 @@ __libc_malloc (size_t bytes)
   if (ar_ptr != NULL)
     __libc_lock_unlock (ar_ptr->mutex);
 
+  victim = TAG_NEW_USABLE (victim);
+
   assert (!victim || chunk_is_mmapped (mem2chunk (victim)) ||
           ar_ptr == arena_for_chunk (mem2chunk (victim)));
   return victim;
@@ -3122,8 +3179,17 @@ __libc_free (void *mem)
   if (mem == 0)                              /* free(0) has no effect */
     return;
 
+#ifdef _LIBC_MTAG
+  /* Quickly check that the freed pointer matches the tag for the memory.
+     This gives a useful double-free detection.  */
+  *(volatile char *)mem;
+#endif
+
   p = mem2chunk (mem);
 
+  /* Mark the chunk as belonging to the library again.  */
+  (void)TAG_REGION (chunk2rawmem (p), __malloc_usable_size (mem));
+
   if (chunk_is_mmapped (p))                       /* release mmapped memory. */
     {
       /* See if the dynamic brk/mmap threshold needs adjusting.
@@ -3173,6 +3239,12 @@ __libc_realloc (void *oldmem, size_t bytes)
   if (oldmem == 0)
     return __libc_malloc (bytes);
 
+#ifdef _LIBC_MTAG
+  /* Perform a quick check to ensure that the pointer's tag matches the
+     memory's tag.  */
+  *(volatile char*) oldmem;
+#endif
+
   /* chunk corresponding to oldmem */
   const mchunkptr oldp = mem2chunk (oldmem);
   /* its size */
@@ -3228,7 +3300,15 @@ __libc_realloc (void *oldmem, size_t bytes)
 #if HAVE_MREMAP
       newp = mremap_chunk (oldp, nb);
       if (newp)
-        return chunk2mem (newp);
+	{
+	  void *newmem = chunk2rawmem (newp);
+	  /* Give the new block a different tag.  This helps to ensure
+	     that stale handles to the previous mapping are not
+	     reused.  There's a performance hit for both us and the
+	     caller for doing this, so we might want to
+	     reconsider.  */
+	  return TAG_NEW_USABLE (newmem);
+	}
 #endif
       /* Note the extra SIZE_SZ overhead. */
       if (oldsize - SIZE_SZ >= nb)
@@ -3311,7 +3391,6 @@ _mid_memalign (size_t alignment, size_t bytes, void *address)
       return 0;
     }
 
-
   /* Make sure alignment is power of 2.  */
   if (!powerof2 (alignment))
     {
@@ -3326,8 +3405,7 @@ _mid_memalign (size_t alignment, size_t bytes, void *address)
       p = _int_memalign (&main_arena, alignment, bytes);
       assert (!p || chunk_is_mmapped (mem2chunk (p)) ||
 	      &main_arena == arena_for_chunk (mem2chunk (p)));
-
-      return p;
+      return TAG_NEW_USABLE (p);
     }
 
   arena_get (ar_ptr, bytes + alignment + MINSIZE);
@@ -3345,7 +3423,7 @@ _mid_memalign (size_t alignment, size_t bytes, void *address)
 
   assert (!p || chunk_is_mmapped (mem2chunk (p)) ||
           ar_ptr == arena_for_chunk (mem2chunk (p)));
-  return p;
+  return TAG_NEW_USABLE (p);
 }
 /* For ISO C11.  */
 weak_alias (__libc_memalign, aligned_alloc)
@@ -3354,17 +3432,22 @@ libc_hidden_def (__libc_memalign)
 void *
 __libc_valloc (size_t bytes)
 {
+  void *p;
+
   if (__malloc_initialized < 0)
     ptmalloc_init ();
 
   void *address = RETURN_ADDRESS (0);
   size_t pagesize = GLRO (dl_pagesize);
-  return _mid_memalign (pagesize, bytes, address);
+  p = _mid_memalign (pagesize, bytes, address);
+  return TAG_NEW_USABLE (p);
 }
 
 void *
 __libc_pvalloc (size_t bytes)
 {
+  void *p;
+
   if (__malloc_initialized < 0)
     ptmalloc_init ();
 
@@ -3381,19 +3464,22 @@ __libc_pvalloc (size_t bytes)
     }
   rounded_bytes = rounded_bytes & -(pagesize - 1);
 
-  return _mid_memalign (pagesize, rounded_bytes, address);
+  p = _mid_memalign (pagesize, rounded_bytes, address);
+  return TAG_NEW_USABLE (p);
 }
 
 void *
 __libc_calloc (size_t n, size_t elem_size)
 {
   mstate av;
-  mchunkptr oldtop, p;
-  INTERNAL_SIZE_T sz, csz, oldtopsize;
+  mchunkptr oldtop;
+  INTERNAL_SIZE_T sz, oldtopsize;
   void *mem;
+#ifndef _LIBC_MTAG
   unsigned long clearsize;
   unsigned long nclears;
   INTERNAL_SIZE_T *d;
+#endif
   ptrdiff_t bytes;
 
   if (__glibc_unlikely (__builtin_mul_overflow (n, elem_size, &bytes)))
@@ -3401,6 +3487,7 @@ __libc_calloc (size_t n, size_t elem_size)
        __set_errno (ENOMEM);
        return NULL;
     }
+
   sz = bytes;
 
   void *(*hook) (size_t, const void *) =
@@ -3470,7 +3557,14 @@ __libc_calloc (size_t n, size_t elem_size)
   if (mem == 0)
     return 0;
 
-  p = mem2chunk (mem);
+  /* If we are using memory tagging, then we need to set the tags
+     regardless of MORECORE_CLEARS, so we zero the whole block while
+     doing so.  */
+#ifdef _LIBC_MTAG
+  return TAG_NEW_MEMSET (mem, 0, __malloc_usable_size (mem));
+#else
+  mchunkptr p = mem2chunk (mem);
+  INTERNAL_SIZE_T csz = chunksize (p);
 
   /* Two optional cases in which clearing not necessary */
   if (chunk_is_mmapped (p))
@@ -3481,8 +3575,6 @@ __libc_calloc (size_t n, size_t elem_size)
       return mem;
     }
 
-  csz = chunksize (p);
-
 #if MORECORE_CLEARS
   if (perturb_byte == 0 && (p == oldtop && csz > oldtopsize))
     {
@@ -3525,6 +3617,7 @@ __libc_calloc (size_t n, size_t elem_size)
     }
 
   return mem;
+#endif
 }
 
 /*
@@ -4621,7 +4714,7 @@ _int_realloc(mstate av, mchunkptr oldp, INTERNAL_SIZE_T oldsize,
           av->top = chunk_at_offset (oldp, nb);
           set_head (av->top, (newsize - nb) | PREV_INUSE);
           check_inuse_chunk (av, oldp);
-          return chunk2mem (oldp);
+          return TAG_NEW_USABLE (chunk2rawmem (oldp));
         }
 
       /* Try to expand forward into next chunk;  split off remainder below */
@@ -4654,7 +4747,10 @@ _int_realloc(mstate av, mchunkptr oldp, INTERNAL_SIZE_T oldsize,
             }
           else
             {
-	      memcpy (newmem, chunk2mem (oldp), oldsize - SIZE_SZ);
+	      void *oldmem = chunk2mem (oldp);
+	      newmem = TAG_NEW_USABLE (newmem);
+	      memcpy (newmem, oldmem, __malloc_usable_size (oldmem));
+	      (void) TAG_REGION (chunk2rawmem (oldp), oldsize);
               _int_free (av, oldp, 1);
               check_inuse_chunk (av, newp);
               return chunk2mem (newp);
@@ -4676,6 +4772,8 @@ _int_realloc(mstate av, mchunkptr oldp, INTERNAL_SIZE_T oldsize,
   else   /* split remainder */
     {
       remainder = chunk_at_offset (newp, nb);
+      /* Clear any user-space tags before writing the header.  */
+      remainder = TAG_REGION (remainder, remainder_size);
       set_head_size (newp, nb | (av != &main_arena ? NON_MAIN_ARENA : 0));
       set_head (remainder, remainder_size | PREV_INUSE |
                 (av != &main_arena ? NON_MAIN_ARENA : 0));
@@ -4685,8 +4783,8 @@ _int_realloc(mstate av, mchunkptr oldp, INTERNAL_SIZE_T oldsize,
     }
 
   check_inuse_chunk (av, newp);
-  return chunk2mem (newp);
-}
+  return TAG_NEW_USABLE (chunk2rawmem (newp));
+    }
 
 /*
    ------------------------------ memalign ------------------------------
@@ -4763,7 +4861,7 @@ _int_memalign (mstate av, size_t alignment, size_t bytes)
       p = newp;
 
       assert (newsize >= nb &&
-              (((unsigned long) (chunk2mem (p))) % alignment) == 0);
+              (((unsigned long) (chunk2rawmem (p))) % alignment) == 0);
     }
 
   /* Also give back spare room at the end */
@@ -4817,7 +4915,7 @@ mtrim (mstate av, size_t pad)
                                                 + sizeof (struct malloc_chunk)
                                                 + psm1) & ~psm1);
 
-                assert ((char *) chunk2mem (p) + 4 * SIZE_SZ <= paligned_mem);
+                assert ((char *) chunk2rawmem (p) + 4 * SIZE_SZ <= paligned_mem);
                 assert ((char *) p + size > paligned_mem);
 
                 /* This is the size we could potentially free.  */
@@ -4880,20 +4978,30 @@ musable (void *mem)
   mchunkptr p;
   if (mem != 0)
     {
+      size_t result = 0;
+
       p = mem2chunk (mem);
 
       if (__builtin_expect (using_malloc_checking == 1, 0))
-        return malloc_check_get_size (p);
+	return malloc_check_get_size (p);
 
       if (chunk_is_mmapped (p))
 	{
 	  if (DUMPED_MAIN_ARENA_CHUNK (p))
-	    return chunksize (p) - SIZE_SZ;
+	    result = chunksize (p) - SIZE_SZ;
 	  else
-	    return chunksize (p) - 2 * SIZE_SZ;
+	    result = chunksize (p) - 2 * SIZE_SZ;
 	}
       else if (inuse (p))
-        return chunksize (p) - SIZE_SZ;
+	result = chunksize (p) - SIZE_SZ;
+
+#ifdef _LIBC_MTAG
+      /* The usable space may be reduced if memory tagging is needed,
+	 since we cannot share the user-space data with malloc's internal
+	 data structure.  */
+      result &= __mtag_granule_mask;
+#endif
+      return result;
     }
   return 0;
 }
diff --git a/malloc/malloc.h b/malloc/malloc.h
index b2371f7704..cf8827e520 100644
--- a/malloc/malloc.h
+++ b/malloc/malloc.h
@@ -77,6 +77,13 @@ extern void *pvalloc (size_t __size) __THROW __attribute_malloc__ __wur;
    contiguous pieces of memory.  */
 extern void *(*__morecore) (ptrdiff_t __size) __MALLOC_DEPRECATED;
 
+#ifdef _LIBC_MTAG
+extern int __mtag_mmap_flags;
+#define MTAG_MMAP_FLAGS __mtag_mmap_flags
+#else
+#define MTAG_MMAP_FLAGS 0
+#endif
+
 /* Default value of `__morecore'.  */
 extern void *__default_morecore (ptrdiff_t __size)
 __THROW __attribute_malloc__  __MALLOC_DEPRECATED;
diff --git a/sysdeps/generic/libc-mtag.h b/sysdeps/generic/libc-mtag.h
new file mode 100644
index 0000000000..3e9885451c
--- /dev/null
+++ b/sysdeps/generic/libc-mtag.h
@@ -0,0 +1,52 @@
+/* libc-internal interface for tagged (colored) memory support.
+   Copyright (C) 2019 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 _GENERIC_LIBC_MTAG_H
+#define _GENERIC_LIBC_MTAG_H 1
+
+/* Generic bindings for systems that do not support memory tagging.  */
+
+/* Used to ensure additional alignment when objects need to have distinct
+   tags.  */
+#define __MTAG_GRANULE_SIZE 1
+
+/* Non-zero if memory obtained via morecore (sbrk) is not tagged.  */
+#define __MTAG_SBRK_UNTAGGED 0
+
+/* Extra flags to pass to mmap() to request a tagged region of memory.  */
+#define __MTAG_MMAP_FLAGS 0
+
+/* Set the tags for a region of memory, which must have size and alignment
+   that are multiples of __MTAG_GRANULE_SIZE.  Size cannot be zero.
+   void *__libc_mtag_tag_region (const void *, size_t)  */
+#define __libc_mtag_tag_region(p, s) (p)
+
+/* Optimized equivalent to __libc_mtag_tag_region followed by memset.  */
+#define __libc_mtag_memset_with_tag memset
+
+/* Convert address P to a pointer that is tagged correctly for that
+   location.
+   void *__libc_mtag_address_get_tag (void*)  */
+#define __libc_mtag_address_get_tag(p) (p)
+
+/* Assign a new (random) tag to a pointer P (does not adjust the tag on
+   the memory addressed).
+   void *__libc_mtag_new_tag (void*)  */
+#define __libc_mtag_new_tag(p) (p)
+
+#endif /* _GENERIC_LIBC_MTAG_H */

Re: [PATCH v3 3/8] malloc: Basic support for memory tagging in the malloc() family

[Florian Weimer]

* Richard Earnshaw via Libc-alpha:

> +/* Set the tags for a region of memory, which must have size and alignment
> +   that are multiples of __MTAG_GRANULE_SIZE.  Size cannot be zero.
> +   void *__libc_mtag_tag_region (const void *, size_t)  */
> +#define __libc_mtag_tag_region(p, s) (p)

I think we need a description of how the tagging operation is supposed
to work.  For example, it's surprising that the requested tag is not
specified in an argument.

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill

Re: [PATCH v3 3/8] malloc: Basic support for memory tagging in the malloc() family

[Richard Earnshaw]

On 25/11/2020 14:58, Florian Weimer via Libc-alpha wrote:
> * Richard Earnshaw via Libc-alpha:
> 
>> +/* Set the tags for a region of memory, which must have size and alignment
>> +   that are multiples of __MTAG_GRANULE_SIZE.  Size cannot be zero.
>> +   void *__libc_mtag_tag_region (const void *, size_t)  */
>> +#define __libc_mtag_tag_region(p, s) (p)
> 
> I think we need a description of how the tagging operation is supposed
> to work.  For example, it's surprising that the requested tag is not
> specified in an argument.
> 
> Thanks,
> Florian
> 

The (new) tag is already encoded in the pointer.  It's (obviously)
implementation specific how that is done.  I'll clarify the comment.

R.

[PATCH v3 4/8] malloc: Clean up commentary

[Richard Earnshaw]

[-- Attachment #1: Type: text/plain, Size: 326 bytes --]


This patch will be merged with its predecessor before being committed, it is
kept separate for now to ease reviewing.
---
 malloc/arena.c              |   8 +++
 malloc/malloc.c             | 112 +++++++++++++++++++++++++++++-------
 sysdeps/generic/libc-mtag.h |   2 +-
 3 files changed, 101 insertions(+), 21 deletions(-)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v3-0004-malloc-Clean-up-commentary.patch --]
[-- Type: text/x-patch; name="v3-0004-malloc-Clean-up-commentary.patch", Size: 8159 bytes --]

diff --git a/malloc/arena.c b/malloc/arena.c
index 835fcc0fb3..e348b10978 100644
--- a/malloc/arena.c
+++ b/malloc/arena.c
@@ -288,6 +288,10 @@ libc_hidden_proto (_dl_open_hook);
 #endif
 
 #ifdef _LIBC_MTAG
+
+/* Generate a new (random) tag value for PTR and tag the memory it
+   points to upto __malloc_usable_size (PTR).  Return the newly tagged
+   pointer.  */
 static void *
 __mtag_tag_new_usable (void *ptr)
 {
@@ -297,6 +301,10 @@ __mtag_tag_new_usable (void *ptr)
   return ptr;
 }
 
+/* Generate a new (random) tag value for PTR, set the tags for the
+   memory to the new tag and initialize the memory contents to VAL.
+   In practice this function will only be called with VAL=0, but we
+   keep this parameter to maintain the same prototype as memset.  */
 static void *
 __mtag_tag_new_memset (void *ptr, int val, size_t size)
 {
diff --git a/malloc/malloc.c b/malloc/malloc.c
index b866e87bc3..deabeb010b 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -381,7 +381,65 @@ __malloc_assert (const char *assertion, const char *file, unsigned int line,
 void * __default_morecore (ptrdiff_t);
 void *(*__morecore)(ptrdiff_t) = __default_morecore;
 
+/* Memory tagging.  */
+
+/* Some systems support the concept of tagging (sometimes known as
+   coloring) memory locations on a fine grained basis.  Each memory
+   location is given a color (normally allocated randomly) and
+   pointers are also colored.  When the pointer is dereferenced, the
+   pointer's color is checked against the memory's color and if they
+   differ the access is faulted (sometimes lazily).
+
+   We use this in glibc by maintaining a single color for the malloc
+   data structures that are interleaved with the user data and then
+   assigning separate colors for each block allocation handed out.  In
+   this way simple buffer overruns will be rapidly detected.  When
+   memory is freed, the memory is recolored back to the glibc default
+   so that simple use-after-free errors can also be detected.
+
+   If memory is reallocated the buffer is recolored even if the
+   address remains the same.  This has a performance impact, but
+   guarantees that the old pointer cannot mistakenly be reused (code
+   that compares old against new will see a mismatch and will then
+   need to behave as though realloc moved the data to a new location).
+
+   Internal API for memory tagging support.
+
+   The aim is to keep the code for memory tagging support as close to
+   the normal APIs in glibc as possible, so that if tagging is not
+   enabled in the library, or is disabled at runtime then standard
+   operations can continue to be used.  Support macros are used to do
+   this:
+
+   void *TAG_NEW_MEMSET (void *ptr, int, val, size_t size)
+
+   Has the same interface as memset(), but additionally allocates a
+   new tag, colors the memory with that tag and returns a pointer that
+   is correctly colored for that location.  The non-tagging version
+   will simply call memset.
+
+   void *TAG_REGION (void *ptr, size_t size)
+
+   Color the region of memory pointed to by PTR and size SIZE with
+   the color of PTR.  Returns the original pointer.
+
+   void *TAG_NEW_USABLE (void *ptr)
+
+   Allocate a new random color and use it to color the region of
+   memory starting at PTR and of size __malloc_usable_size() with that
+   color.  Returns PTR suitably recolored for accessing the memory there.
+
+   void *TAG_AT (void *ptr)
+
+   Read the current color of the memory at the address pointed to by
+   PTR (ignoring it's current color) and return PTR recolored to that
+   color.  PTR must be valid address in all other respects.  When
+   tagging is not enabled, it simply returns the original pointer.
+*/
+
 #ifdef _LIBC_MTAG
+
+/* Default implementaions when memory tagging is supported, but disabled.  */
 static void *
 __default_tag_region (void *ptr, size_t size)
 {
@@ -413,21 +471,6 @@ static void *(*__tag_at)(void *) = __default_tag_nop;
 # define TAG_AT(ptr) (ptr)
 #endif
 
-/* When using tagged memory, we cannot share the end of the user block
-   with the header for the next chunk, so ensure that we allocate
-   blocks that are rounded up to the granule size.  Take care not to
-   overflow from close to MAX_SIZE_T to a small number.  */
-static inline size_t
-ROUND_UP_ALLOCATION_SIZE(size_t bytes)
-{
-#ifdef _LIBC_MTAG
-  return (bytes + ~__mtag_granule_mask) & __mtag_granule_mask;
-#else
-  return bytes;
-#endif
-}
-
-
 #include <string.h>
 
 /*
@@ -1234,10 +1277,26 @@ nextchunk-> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   ---------- Size and alignment checks and conversions ----------
 */
 
-/* conversion from malloc headers to user pointers, and back */
+/* Conversion from malloc headers to user pointers, and back.  When
+   using memory tagging the user data and the malloc data structure
+   headers have distinct tags.  Converting fully from one to the other
+   involves extracting the tag at the other address and creating a
+   suitable pointer using it.  That can be quite expensive.  There are
+   many occasions, though when the pointer will not be dereferenced
+   (for example, because we only want to assert that the pointer is
+   correctly aligned).  In these cases it is more efficient not
+   to extract the tag, since the answer will be the same either way.
+   chunk2rawmem() can be used in these cases.
+ */
 
-#define chunk2mem(p) ((void*)TAG_AT (((char*)(p) + 2*SIZE_SZ)))
+/* Convert a user mem pointer to a chunk address without correcting
+   the tag.  */
 #define chunk2rawmem(p) ((void*)((char*)(p) + 2*SIZE_SZ))
+
+/* Convert between user mem pointers and chunk pointers, updating any
+   memory tags on the pointer to respect the tag value at that
+   location.  */
+#define chunk2mem(p) ((void*)TAG_AT (((char*)(p) + 2*SIZE_SZ)))
 #define mem2chunk(mem) ((mchunkptr)TAG_AT (((char*)(mem) - 2*SIZE_SZ)))
 
 /* The smallest possible chunk */
@@ -1257,7 +1316,8 @@ nextchunk-> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
    & MALLOC_ALIGN_MASK)
 
 /* pad request bytes into a usable size -- internal version */
-
+/* Note: This must be a macro that evaluates to a compile time constant
+   if passed a literal constant.  */
 #define request2size(req)                                         \
   (((req) + SIZE_SZ + MALLOC_ALIGN_MASK < MINSIZE)  ?             \
    MINSIZE :                                                      \
@@ -1272,7 +1332,18 @@ checked_request2size (size_t req, size_t *sz) __nonnull (1)
 {
   if (__glibc_unlikely (req > PTRDIFF_MAX))
     return false;
-  req = ROUND_UP_ALLOCATION_SIZE (req);
+
+#ifdef _LIBC_MTAG
+  /* When using tagged memory, we cannot share the end of the user
+     block with the header for the next chunk, so ensure that we
+     allocate blocks that are rounded up to the granule size.  Take
+     care not to overflow from close to MAX_SIZE_T to a small
+     number.  Ideally, this would be part of request2size(), but that
+     must be a macro that produces a compile time constant if passed
+     a constant literal.  */
+  req = (req + ~__mtag_granule_mask) & __mtag_granule_mask;
+#endif
+
   *sz = request2size (req);
   return true;
 }
@@ -3391,6 +3462,7 @@ _mid_memalign (size_t alignment, size_t bytes, void *address)
       return 0;
     }
 
+
   /* Make sure alignment is power of 2.  */
   if (!powerof2 (alignment))
     {
@@ -4784,7 +4856,7 @@ _int_realloc(mstate av, mchunkptr oldp, INTERNAL_SIZE_T oldsize,
 
   check_inuse_chunk (av, newp);
   return TAG_NEW_USABLE (chunk2rawmem (newp));
-    }
+}
 
 /*
    ------------------------------ memalign ------------------------------
diff --git a/sysdeps/generic/libc-mtag.h b/sysdeps/generic/libc-mtag.h
index 3e9885451c..07f0203253 100644
--- a/sysdeps/generic/libc-mtag.h
+++ b/sysdeps/generic/libc-mtag.h
@@ -1,5 +1,5 @@
 /* libc-internal interface for tagged (colored) memory support.
-   Copyright (C) 2019 Free Software Foundation, Inc.
+   Copyright (C) 2020 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

[PATCH v3 5/8] malloc: support MALLOC_CHECK_ in conjunction with _MTAG_ENABLE.

[Richard Earnshaw]

[-- Attachment #1: Type: text/plain, Size: 369 bytes --]


Note, I propose that this patch will be merged into the main malloc
changes before committing, it is kept separate here to simplify
reviewing.
---
 malloc/arena.c  |  16 ++++++--
 malloc/hooks.c  |  85 ++++++++++++++++++++++++---------------
 malloc/malloc.c | 104 +++++++++++++++++++++++++++++-------------------
 3 files changed, 127 insertions(+), 78 deletions(-)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v3-0005-malloc-support-MALLOC_CHECK_-in-conjunction-with-.patch --]
[-- Type: text/x-patch; name="v3-0005-malloc-support-MALLOC_CHECK_-in-conjunction-with-.patch", Size: 23174 bytes --]

diff --git a/malloc/arena.c b/malloc/arena.c
index e348b10978..da4bdc9d51 100644
--- a/malloc/arena.c
+++ b/malloc/arena.c
@@ -290,14 +290,22 @@ libc_hidden_proto (_dl_open_hook);
 #ifdef _LIBC_MTAG
 
 /* Generate a new (random) tag value for PTR and tag the memory it
-   points to upto __malloc_usable_size (PTR).  Return the newly tagged
-   pointer.  */
+   points to upto the end of the usable size for the chunk containing
+   it.  Return the newly tagged pointer.  */
 static void *
 __mtag_tag_new_usable (void *ptr)
 {
   if (ptr)
-    ptr = __libc_mtag_tag_region (__libc_mtag_new_tag (ptr),
-				  __malloc_usable_size (ptr));
+    {
+      mchunkptr cp = mem2chunk(ptr);
+      /* This likely will never happen, but we can't handle retagging
+	 chunks from the dumped main arena.  So just return the
+	 existing pointer.  */
+      if (DUMPED_MAIN_ARENA_CHUNK (cp))
+	return ptr;
+      ptr = __libc_mtag_tag_region (__libc_mtag_new_tag (ptr),
+				    CHUNK_AVAILABLE_SIZE (cp) - CHUNK_HDR_SZ);
+    }
   return ptr;
 }
 
diff --git a/malloc/hooks.c b/malloc/hooks.c
index 52bb3863cd..c6d7ed774f 100644
--- a/malloc/hooks.c
+++ b/malloc/hooks.c
@@ -56,12 +56,6 @@ static int using_malloc_checking;
 void
 __malloc_check_init (void)
 {
-#if HAVE_TUNABLES && defined (_LIBC_MTAG)
-  /* If memory tagging is enabled, there is no need for the boundary
-     checking cookie as well.  */
-  if ((TUNABLE_GET_FULL (glibc, memtag, enable, int32_t, NULL) & 1) != 0)
-    return;
-#endif
   using_malloc_checking = 1;
   __malloc_hook = malloc_check;
   __free_hook = free_check;
@@ -69,6 +63,13 @@ __malloc_check_init (void)
   __memalign_hook = memalign_check;
 }
 
+/* When memory is tagged, the checking data is stored in the user part
+   of the chunk.  We can't rely on the user not having modified the
+   tags, so fetch the tag at each location before dereferencing
+   it.  */
+#define SAFE_CHAR_OFFSET(p,offset) \
+  ((unsigned char *) TAG_AT (((unsigned char *) p) + offset))
+
 /* A simple, standard set of debugging hooks.  Overhead is `only' one
    byte per chunk; still this will catch most cases of double frees or
    overruns.  The goal here is to avoid obscure crashes due to invalid
@@ -86,7 +87,6 @@ magicbyte (const void *p)
   return magic;
 }
 
-
 /* Visualize the chunk as being partitioned into blocks of 255 bytes from the
    highest address of the chunk, downwards.  The end of each block tells
    us the size of that block, up to the actual size of the requested
@@ -102,16 +102,16 @@ malloc_check_get_size (mchunkptr p)
 
   assert (using_malloc_checking == 1);
 
-  for (size = chunksize (p) - 1 + (chunk_is_mmapped (p) ? 0 : SIZE_SZ);
-       (c = ((unsigned char *) p)[size]) != magic;
+  for (size = CHUNK_AVAILABLE_SIZE (p) - 1;
+       (c = *SAFE_CHAR_OFFSET (p, size)) != magic;
        size -= c)
     {
-      if (c <= 0 || size < (c + 2 * SIZE_SZ))
+      if (c <= 0 || size < (c + CHUNK_HDR_SZ))
 	malloc_printerr ("malloc_check_get_size: memory corruption");
     }
 
   /* chunk2mem size.  */
-  return size - 2 * SIZE_SZ;
+  return size - CHUNK_HDR_SZ;
 }
 
 /* Instrument a chunk with overrun detector byte(s) and convert it
@@ -130,9 +130,8 @@ mem2mem_check (void *ptr, size_t req_sz)
 
   p = mem2chunk (ptr);
   magic = magicbyte (p);
-  max_sz = chunksize (p) - 2 * SIZE_SZ;
-  if (!chunk_is_mmapped (p))
-    max_sz += SIZE_SZ;
+  max_sz = CHUNK_AVAILABLE_SIZE (p) - CHUNK_HDR_SZ;
+
   for (i = max_sz - 1; i > req_sz; i -= block_sz)
     {
       block_sz = MIN (i - req_sz, 0xff);
@@ -141,9 +140,9 @@ mem2mem_check (void *ptr, size_t req_sz)
       if (block_sz == magic)
         --block_sz;
 
-      m_ptr[i] = block_sz;
+      *SAFE_CHAR_OFFSET (m_ptr, i) = block_sz;
     }
-  m_ptr[req_sz] = magic;
+  *SAFE_CHAR_OFFSET (m_ptr, req_sz) = magic;
   return (void *) m_ptr;
 }
 
@@ -176,9 +175,11 @@ mem2chunk_check (void *mem, unsigned char **magic_p)
                                next_chunk (prev_chunk (p)) != p)))
         return NULL;
 
-      for (sz += SIZE_SZ - 1; (c = ((unsigned char *) p)[sz]) != magic; sz -= c)
+      for (sz = CHUNK_AVAILABLE_SIZE (p) - 1;
+	   (c = *SAFE_CHAR_OFFSET (p, sz)) != magic;
+	   sz -= c)
         {
-          if (c == 0 || sz < (c + 2 * SIZE_SZ))
+          if (c == 0 || sz < (c + CHUNK_HDR_SZ))
             return NULL;
         }
     }
@@ -199,15 +200,19 @@ mem2chunk_check (void *mem, unsigned char **magic_p)
           ((prev_size (p) + sz) & page_mask) != 0)
         return NULL;
 
-      for (sz -= 1; (c = ((unsigned char *) p)[sz]) != magic; sz -= c)
+      for (sz = CHUNK_AVAILABLE_SIZE (p) - 1;
+	   (c = *SAFE_CHAR_OFFSET (p, sz)) != magic;
+	   sz -= c)
         {
-          if (c == 0 || sz < (c + 2 * SIZE_SZ))
+          if (c == 0 || sz < (c + CHUNK_HDR_SZ))
             return NULL;
         }
     }
-  ((unsigned char *) p)[sz] ^= 0xFF;
+
+  unsigned char* safe_p = SAFE_CHAR_OFFSET (p, sz);
+  *safe_p ^= 0xFF;
   if (magic_p)
-    *magic_p = (unsigned char *) p + sz;
+    *magic_p = safe_p;
   return p;
 }
 
@@ -244,7 +249,7 @@ malloc_check (size_t sz, const void *caller)
   top_check ();
   victim = _int_malloc (&main_arena, nb);
   __libc_lock_unlock (main_arena.mutex);
-  return mem2mem_check (victim, sz);
+  return mem2mem_check (TAG_NEW_USABLE (victim), sz);
 }
 
 static void
@@ -255,6 +260,12 @@ free_check (void *mem, const void *caller)
   if (!mem)
     return;
 
+#ifdef _LIBC_MTAG
+  /* Quickly check that the freed pointer matches the tag for the memory.
+     This gives a useful double-free detection.  */
+  *(volatile char *)mem;
+#endif
+
   __libc_lock_lock (main_arena.mutex);
   p = mem2chunk_check (mem, NULL);
   if (!p)
@@ -265,6 +276,8 @@ free_check (void *mem, const void *caller)
       munmap_chunk (p);
       return;
     }
+  /* Mark the chunk as belonging to the library again.  */
+  (void)TAG_REGION (chunk2rawmem (p), CHUNK_AVAILABLE_SIZE (p) - CHUNK_HDR_SZ);
   _int_free (&main_arena, p, 1);
   __libc_lock_unlock (main_arena.mutex);
 }
@@ -272,7 +285,7 @@ free_check (void *mem, const void *caller)
 static void *
 realloc_check (void *oldmem, size_t bytes, const void *caller)
 {
-  INTERNAL_SIZE_T nb;
+  INTERNAL_SIZE_T chnb;
   void *newmem = 0;
   unsigned char *magic_p;
   size_t rb;
@@ -290,14 +303,21 @@ realloc_check (void *oldmem, size_t bytes, const void *caller)
       free_check (oldmem, NULL);
       return NULL;
     }
+
+#ifdef _LIBC_MTAG
+  /* Quickly check that the freed pointer matches the tag for the memory.
+     This gives a useful double-free detection.  */
+  *(volatile char *)oldmem;
+#endif
+
   __libc_lock_lock (main_arena.mutex);
   const mchunkptr oldp = mem2chunk_check (oldmem, &magic_p);
   __libc_lock_unlock (main_arena.mutex);
   if (!oldp)
     malloc_printerr ("realloc(): invalid pointer");
-  const INTERNAL_SIZE_T oldsize = chunksize (oldp);
+  const INTERNAL_SIZE_T oldchsize = CHUNK_AVAILABLE_SIZE (oldp);
 
-  if (!checked_request2size (rb, &nb))
+  if (!checked_request2size (rb, &chnb))
     goto invert;
 
   __libc_lock_lock (main_arena.mutex);
@@ -305,14 +325,13 @@ realloc_check (void *oldmem, size_t bytes, const void *caller)
   if (chunk_is_mmapped (oldp))
     {
 #if HAVE_MREMAP
-      mchunkptr newp = mremap_chunk (oldp, nb);
+      mchunkptr newp = mremap_chunk (oldp, chnb);
       if (newp)
         newmem = chunk2mem (newp);
       else
 #endif
       {
-        /* Note the extra SIZE_SZ overhead. */
-        if (oldsize - SIZE_SZ >= nb)
+        if (oldchsize >= chnb)
           newmem = oldmem; /* do nothing */
         else
           {
@@ -321,7 +340,7 @@ realloc_check (void *oldmem, size_t bytes, const void *caller)
 	    newmem = _int_malloc (&main_arena, rb);
             if (newmem)
               {
-                memcpy (newmem, oldmem, oldsize - 2 * SIZE_SZ);
+                memcpy (newmem, oldmem, oldchsize - CHUNK_HDR_SZ);
                 munmap_chunk (oldp);
               }
           }
@@ -330,7 +349,7 @@ realloc_check (void *oldmem, size_t bytes, const void *caller)
   else
     {
       top_check ();
-      newmem = _int_realloc (&main_arena, oldp, oldsize, nb);
+      newmem = _int_realloc (&main_arena, oldp, oldchsize, chnb);
     }
 
   DIAG_PUSH_NEEDS_COMMENT;
@@ -349,7 +368,7 @@ invert:
 
   __libc_lock_unlock (main_arena.mutex);
 
-  return mem2mem_check (newmem, bytes);
+  return mem2mem_check (TAG_NEW_USABLE (newmem), bytes);
 }
 
 static void *
@@ -391,7 +410,7 @@ memalign_check (size_t alignment, size_t bytes, const void *caller)
   top_check ();
   mem = _int_memalign (&main_arena, alignment, bytes + 1);
   __libc_lock_unlock (main_arena.mutex);
-  return mem2mem_check (mem, bytes);
+  return mem2mem_check (TAG_NEW_USABLE (mem), bytes);
 }
 
 #if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_25)
diff --git a/malloc/malloc.c b/malloc/malloc.c
index deabeb010b..bd78d4bbc6 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -425,9 +425,10 @@ void *(*__morecore)(ptrdiff_t) = __default_morecore;
 
    void *TAG_NEW_USABLE (void *ptr)
 
-   Allocate a new random color and use it to color the region of
-   memory starting at PTR and of size __malloc_usable_size() with that
-   color.  Returns PTR suitably recolored for accessing the memory there.
+   Allocate a new random color and use it to color the user region of
+   a chunk; this may include data from the subsequent chunk's header
+   if tagging is sufficiently fine grained.  Returns PTR suitably
+   recolored for accessing the memory there.
 
    void *TAG_AT (void *ptr)
 
@@ -1289,15 +1290,19 @@ nextchunk-> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
    chunk2rawmem() can be used in these cases.
  */
 
+/* The chunk header is two SIZE_SZ elements, but this is used widely, so
+   we define it here for clarity later.  */
+#define CHUNK_HDR_SZ (2 * SIZE_SZ)
+
 /* Convert a user mem pointer to a chunk address without correcting
    the tag.  */
-#define chunk2rawmem(p) ((void*)((char*)(p) + 2*SIZE_SZ))
+#define chunk2rawmem(p) ((void*)((char*)(p) + CHUNK_HDR_SZ))
 
 /* Convert between user mem pointers and chunk pointers, updating any
    memory tags on the pointer to respect the tag value at that
    location.  */
-#define chunk2mem(p) ((void*)TAG_AT (((char*)(p) + 2*SIZE_SZ)))
-#define mem2chunk(mem) ((mchunkptr)TAG_AT (((char*)(mem) - 2*SIZE_SZ)))
+#define chunk2mem(p) ((void*)TAG_AT (((char*)(p) + CHUNK_HDR_SZ)))
+#define mem2chunk(mem) ((mchunkptr)TAG_AT (((char*)(mem) - CHUNK_HDR_SZ)))
 
 /* The smallest possible chunk */
 #define MIN_CHUNK_SIZE        (offsetof(struct malloc_chunk, fd_nextsize))
@@ -1312,7 +1317,7 @@ nextchunk-> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
 #define aligned_OK(m)  (((unsigned long)(m) & MALLOC_ALIGN_MASK) == 0)
 
 #define misaligned_chunk(p) \
-  ((uintptr_t)(MALLOC_ALIGNMENT == 2 * SIZE_SZ ? (p) : chunk2mem (p)) \
+  ((uintptr_t)(MALLOC_ALIGNMENT == CHUNK_HDR_SZ ? (p) : chunk2mem (p)) \
    & MALLOC_ALIGN_MASK)
 
 /* pad request bytes into a usable size -- internal version */
@@ -1323,6 +1328,17 @@ nextchunk-> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
    MINSIZE :                                                      \
    ((req) + SIZE_SZ + MALLOC_ALIGN_MASK) & ~MALLOC_ALIGN_MASK)
 
+/* Available size of chunk.  This is the size of the real usable data
+   in the chunk, plus the chunk header.  */
+#ifdef _LIBC_MTAG
+#define CHUNK_AVAILABLE_SIZE(p) \
+  ((chunksize (p) + (chunk_is_mmapped (p) ? 0 : SIZE_SZ))	\
+   & __mtag_granule_mask)
+#else
+#define CHUNK_AVAILABLE_SIZE(p) \
+  (chunksize (p) + (chunk_is_mmapped (p) ? 0 : SIZE_SZ))
+#endif
+
 /* Check if REQ overflows when padded and aligned and if the resulting value
    is less than PTRDIFF_T.  Returns TRUE and the requested size or MINSIZE in
    case the value is less than MINSIZE on SZ or false if any of the previous
@@ -1442,7 +1458,6 @@ checked_request2size (size_t req, size_t *sz) __nonnull (1)
 /* Set size at footer (only when chunk is not in use) */
 #define set_foot(p, s)       (((mchunkptr) ((char *) (p) + (s)))->mchunk_prev_size = (s))
 
-
 #pragma GCC poison mchunk_size
 #pragma GCC poison mchunk_prev_size
 
@@ -1538,7 +1553,7 @@ typedef struct malloc_chunk *mbinptr;
 #define NBINS             128
 #define NSMALLBINS         64
 #define SMALLBIN_WIDTH    MALLOC_ALIGNMENT
-#define SMALLBIN_CORRECTION (MALLOC_ALIGNMENT > 2 * SIZE_SZ)
+#define SMALLBIN_CORRECTION (MALLOC_ALIGNMENT > CHUNK_HDR_SZ)
 #define MIN_LARGE_SIZE    ((NSMALLBINS - SMALLBIN_CORRECTION) * SMALLBIN_WIDTH)
 
 #define in_smallbin_range(sz)  \
@@ -2438,7 +2453,7 @@ sysmalloc (INTERNAL_SIZE_T nb, mstate av)
          See the front_misalign handling below, for glibc there is no
          need for further alignments unless we have have high alignment.
        */
-      if (MALLOC_ALIGNMENT == 2 * SIZE_SZ)
+      if (MALLOC_ALIGNMENT == CHUNK_HDR_SZ)
         size = ALIGN_UP (nb + SIZE_SZ, pagesize);
       else
         size = ALIGN_UP (nb + SIZE_SZ + MALLOC_ALIGN_MASK, pagesize);
@@ -2460,11 +2475,13 @@ sysmalloc (INTERNAL_SIZE_T nb, mstate av)
                  address argument for later munmap in free() and realloc().
                */
 
-              if (MALLOC_ALIGNMENT == 2 * SIZE_SZ)
+              if (MALLOC_ALIGNMENT == CHUNK_HDR_SZ)
                 {
-                  /* For glibc, chunk2rawmem increases the address by 2*SIZE_SZ and
-                     MALLOC_ALIGN_MASK is 2*SIZE_SZ-1.  Each mmap'ed area is page
-                     aligned and therefore definitely MALLOC_ALIGN_MASK-aligned.  */
+                  /* For glibc, chunk2rawmem increases the address by
+                     CHUNK_HDR_SZ and MALLOC_ALIGN_MASK is
+                     CHUNK_HDR_SZ-1.  Each mmap'ed area is page
+                     aligned and therefore definitely
+                     MALLOC_ALIGN_MASK-aligned.  */
                   assert (((INTERNAL_SIZE_T) chunk2rawmem (mm) & MALLOC_ALIGN_MASK) == 0);
                   front_misalign = 0;
                 }
@@ -2557,18 +2574,20 @@ sysmalloc (INTERNAL_SIZE_T nb, mstate av)
              become the top chunk again later.  Note that a footer is set
              up, too, although the chunk is marked in use. */
           old_size = (old_size - MINSIZE) & ~MALLOC_ALIGN_MASK;
-          set_head (chunk_at_offset (old_top, old_size + 2 * SIZE_SZ), 0 | PREV_INUSE);
+          set_head (chunk_at_offset (old_top, old_size + CHUNK_HDR_SZ),
+		    0 | PREV_INUSE);
           if (old_size >= MINSIZE)
             {
-              set_head (chunk_at_offset (old_top, old_size), (2 * SIZE_SZ) | PREV_INUSE);
-              set_foot (chunk_at_offset (old_top, old_size), (2 * SIZE_SZ));
+              set_head (chunk_at_offset (old_top, old_size),
+			CHUNK_HDR_SZ | PREV_INUSE);
+              set_foot (chunk_at_offset (old_top, old_size), CHUNK_HDR_SZ);
               set_head (old_top, old_size | PREV_INUSE | NON_MAIN_ARENA);
               _int_free (av, old_top, 1);
             }
           else
             {
-              set_head (old_top, (old_size + 2 * SIZE_SZ) | PREV_INUSE);
-              set_foot (old_top, (old_size + 2 * SIZE_SZ));
+              set_head (old_top, (old_size + CHUNK_HDR_SZ) | PREV_INUSE);
+              set_foot (old_top, (old_size + CHUNK_HDR_SZ));
             }
         }
       else if (!tried_mmap)
@@ -2770,7 +2789,7 @@ sysmalloc (INTERNAL_SIZE_T nb, mstate av)
               /* handle non-contiguous cases */
               else
                 {
-                  if (MALLOC_ALIGNMENT == 2 * SIZE_SZ)
+                  if (MALLOC_ALIGNMENT == CHUNK_HDR_SZ)
                     /* MORECORE/mmap must correctly align */
                     assert (((unsigned long) chunk2rawmem (brk) & MALLOC_ALIGN_MASK) == 0);
                   else
@@ -2820,7 +2839,7 @@ sysmalloc (INTERNAL_SIZE_T nb, mstate av)
                          multiple of MALLOC_ALIGNMENT. We know there is at least
                          enough space in old_top to do this.
                        */
-                      old_size = (old_size - 4 * SIZE_SZ) & ~MALLOC_ALIGN_MASK;
+                      old_size = (old_size - 2 * CHUNK_HDR_SZ) & ~MALLOC_ALIGN_MASK;
                       set_head (old_top, old_size | PREV_INUSE);
 
                       /*
@@ -2830,9 +2849,10 @@ sysmalloc (INTERNAL_SIZE_T nb, mstate av)
                          lost.
                        */
 		      set_head (chunk_at_offset (old_top, old_size),
-				(2 * SIZE_SZ) | PREV_INUSE);
-		      set_head (chunk_at_offset (old_top, old_size + 2 * SIZE_SZ),
-				(2 * SIZE_SZ) | PREV_INUSE);
+				CHUNK_HDR_SZ | PREV_INUSE);
+		      set_head (chunk_at_offset (old_top,
+						 old_size + CHUNK_HDR_SZ),
+				CHUNK_HDR_SZ | PREV_INUSE);
 
                       /* If possible, release the rest. */
                       if (old_size >= MINSIZE)
@@ -3259,7 +3279,7 @@ __libc_free (void *mem)
   p = mem2chunk (mem);
 
   /* Mark the chunk as belonging to the library again.  */
-  (void)TAG_REGION (chunk2rawmem (p), __malloc_usable_size (mem));
+  (void)TAG_REGION (chunk2rawmem (p), CHUNK_AVAILABLE_SIZE (p) - CHUNK_HDR_SZ);
 
   if (chunk_is_mmapped (p))                       /* release mmapped memory. */
     {
@@ -3358,7 +3378,7 @@ __libc_realloc (void *oldmem, size_t bytes)
 	    return NULL;
 	  /* Copy as many bytes as are available from the old chunk
 	     and fit into the new size.  NB: The overhead for faked
-	     mmapped chunks is only SIZE_SZ, not 2 * SIZE_SZ as for
+	     mmapped chunks is only SIZE_SZ, not CHUNK_HDR_SZ as for
 	     regular mmapped chunks.  */
 	  if (bytes > oldsize - SIZE_SZ)
 	    bytes = oldsize - SIZE_SZ;
@@ -3390,7 +3410,7 @@ __libc_realloc (void *oldmem, size_t bytes)
       if (newmem == 0)
         return 0;              /* propagate failure */
 
-      memcpy (newmem, oldmem, oldsize - 2 * SIZE_SZ);
+      memcpy (newmem, oldmem, oldsize - CHUNK_HDR_SZ);
       munmap_chunk (oldp);
       return newmem;
     }
@@ -3629,13 +3649,13 @@ __libc_calloc (size_t n, size_t elem_size)
   if (mem == 0)
     return 0;
 
+  mchunkptr p = mem2chunk (mem);
   /* If we are using memory tagging, then we need to set the tags
      regardless of MORECORE_CLEARS, so we zero the whole block while
      doing so.  */
 #ifdef _LIBC_MTAG
-  return TAG_NEW_MEMSET (mem, 0, __malloc_usable_size (mem));
+  return TAG_NEW_MEMSET (mem, 0, CHUNK_AVAILABLE_SIZE (p) - CHUNK_HDR_SZ);
 #else
-  mchunkptr p = mem2chunk (mem);
   INTERNAL_SIZE_T csz = chunksize (p);
 
   /* Two optional cases in which clearing not necessary */
@@ -3927,10 +3947,10 @@ _int_malloc (mstate av, size_t bytes)
           size = chunksize (victim);
           mchunkptr next = chunk_at_offset (victim, size);
 
-          if (__glibc_unlikely (size <= 2 * SIZE_SZ)
+          if (__glibc_unlikely (size <= CHUNK_HDR_SZ)
               || __glibc_unlikely (size > av->system_mem))
             malloc_printerr ("malloc(): invalid size (unsorted)");
-          if (__glibc_unlikely (chunksize_nomask (next) < 2 * SIZE_SZ)
+          if (__glibc_unlikely (chunksize_nomask (next) < CHUNK_HDR_SZ)
               || __glibc_unlikely (chunksize_nomask (next) > av->system_mem))
             malloc_printerr ("malloc(): invalid next size (unsorted)");
           if (__glibc_unlikely ((prev_size (next) & ~(SIZE_BITS)) != size))
@@ -4429,7 +4449,7 @@ _int_free (mstate av, mchunkptr p, int have_lock)
       ) {
 
     if (__builtin_expect (chunksize_nomask (chunk_at_offset (p, size))
-			  <= 2 * SIZE_SZ, 0)
+			  <= CHUNK_HDR_SZ, 0)
 	|| __builtin_expect (chunksize (chunk_at_offset (p, size))
 			     >= av->system_mem, 0))
       {
@@ -4440,7 +4460,7 @@ _int_free (mstate av, mchunkptr p, int have_lock)
 	if (!have_lock)
 	  {
 	    __libc_lock_lock (av->mutex);
-	    fail = (chunksize_nomask (chunk_at_offset (p, size)) <= 2 * SIZE_SZ
+	    fail = (chunksize_nomask (chunk_at_offset (p, size)) <= CHUNK_HDR_SZ
 		    || chunksize (chunk_at_offset (p, size)) >= av->system_mem);
 	    __libc_lock_unlock (av->mutex);
 	  }
@@ -4449,7 +4469,7 @@ _int_free (mstate av, mchunkptr p, int have_lock)
 	  malloc_printerr ("free(): invalid next size (fast)");
       }
 
-    free_perturb (chunk2mem(p), size - 2 * SIZE_SZ);
+    free_perturb (chunk2mem(p), size - CHUNK_HDR_SZ);
 
     atomic_store_relaxed (&av->have_fastchunks, true);
     unsigned int idx = fastbin_index(size);
@@ -4518,11 +4538,11 @@ _int_free (mstate av, mchunkptr p, int have_lock)
       malloc_printerr ("double free or corruption (!prev)");
 
     nextsize = chunksize(nextchunk);
-    if (__builtin_expect (chunksize_nomask (nextchunk) <= 2 * SIZE_SZ, 0)
+    if (__builtin_expect (chunksize_nomask (nextchunk) <= CHUNK_HDR_SZ, 0)
 	|| __builtin_expect (nextsize >= av->system_mem, 0))
       malloc_printerr ("free(): invalid next size (normal)");
 
-    free_perturb (chunk2mem(p), size - 2 * SIZE_SZ);
+    free_perturb (chunk2mem(p), size - CHUNK_HDR_SZ);
 
     /* consolidate backward */
     if (!prev_inuse(p)) {
@@ -4753,7 +4773,7 @@ _int_realloc(mstate av, mchunkptr oldp, INTERNAL_SIZE_T oldsize,
   unsigned long    remainder_size;  /* its size */
 
   /* oldmem size */
-  if (__builtin_expect (chunksize_nomask (oldp) <= 2 * SIZE_SZ, 0)
+  if (__builtin_expect (chunksize_nomask (oldp) <= CHUNK_HDR_SZ, 0)
       || __builtin_expect (oldsize >= av->system_mem, 0))
     malloc_printerr ("realloc(): invalid old size");
 
@@ -4764,7 +4784,7 @@ _int_realloc(mstate av, mchunkptr oldp, INTERNAL_SIZE_T oldsize,
 
   next = chunk_at_offset (oldp, oldsize);
   INTERNAL_SIZE_T nextsize = chunksize (next);
-  if (__builtin_expect (chunksize_nomask (next) <= 2 * SIZE_SZ, 0)
+  if (__builtin_expect (chunksize_nomask (next) <= CHUNK_HDR_SZ, 0)
       || __builtin_expect (nextsize >= av->system_mem, 0))
     malloc_printerr ("realloc(): invalid next size");
 
@@ -4821,7 +4841,8 @@ _int_realloc(mstate av, mchunkptr oldp, INTERNAL_SIZE_T oldsize,
             {
 	      void *oldmem = chunk2mem (oldp);
 	      newmem = TAG_NEW_USABLE (newmem);
-	      memcpy (newmem, oldmem, __malloc_usable_size (oldmem));
+	      memcpy (newmem, oldmem,
+		      CHUNK_AVAILABLE_SIZE (oldp) - CHUNK_HDR_SZ);
 	      (void) TAG_REGION (chunk2rawmem (oldp), oldsize);
               _int_free (av, oldp, 1);
               check_inuse_chunk (av, newp);
@@ -4987,7 +5008,8 @@ mtrim (mstate av, size_t pad)
                                                 + sizeof (struct malloc_chunk)
                                                 + psm1) & ~psm1);
 
-                assert ((char *) chunk2rawmem (p) + 4 * SIZE_SZ <= paligned_mem);
+                assert ((char *) chunk2rawmem (p) + 2 * CHUNK_HDR_SZ
+			<= paligned_mem);
                 assert ((char *) p + size > paligned_mem);
 
                 /* This is the size we could potentially free.  */
@@ -5062,7 +5084,7 @@ musable (void *mem)
 	  if (DUMPED_MAIN_ARENA_CHUNK (p))
 	    result = chunksize (p) - SIZE_SZ;
 	  else
-	    result = chunksize (p) - 2 * SIZE_SZ;
+	    result = chunksize (p) - CHUNK_HDR_SZ;
 	}
       else if (inuse (p))
 	result = chunksize (p) - SIZE_SZ;

[PATCH v3 6/8] linux: Add compatibility definitions to sys/prctl.h for MTE

[Richard Earnshaw]

[-- Attachment #1: Type: text/plain, Size: 432 bytes --]


Older versions of the Linux kernel headers obviously lack support for
memory tagging, but we still want to be able to build in support when
using those (obviously it can't be enabled on such systems).

The linux kernel extensions are made to the platform-independent
header (linux/prctl.h), so this patch takes a similar approach.
---
 sysdeps/unix/sysv/linux/sys/prctl.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v3-0006-linux-Add-compatibility-definitions-to-sys-prctl..patch --]
[-- Type: text/x-patch; name="v3-0006-linux-Add-compatibility-definitions-to-sys-prctl..patch", Size: 971 bytes --]

diff --git a/sysdeps/unix/sysv/linux/sys/prctl.h b/sysdeps/unix/sysv/linux/sys/prctl.h
index 7f748ebeeb..4d01379c23 100644
--- a/sysdeps/unix/sysv/linux/sys/prctl.h
+++ b/sysdeps/unix/sysv/linux/sys/prctl.h
@@ -21,6 +21,24 @@
 #include <features.h>
 #include <linux/prctl.h>  /*  The magic values come from here  */
 
+/* Recent extensions to linux which may post-date the kernel headers
+   we're picking up...  */
+
+/* Memory tagging control operations (for AArch64).  */
+#ifndef PR_TAGGED_ADDR_ENABLE
+# define PR_TAGGED_ADDR_ENABLE	(1UL << 8)
+#endif
+
+#ifndef PR_MTE_TCF_SHIFT
+# define PR_MTE_TCF_SHIFT	1
+# define PR_MTE_TCF_NONE	(0UL << PR_MTE_TCF_SHIFT)
+# define PR_MTE_TCF_SYNC	(1UL << PR_MTE_TCF_SHIFT)
+# define PR_MTE_TCF_ASYNC	(2UL << PR_MTE_TCF_SHIFT)
+# define PR_MTE_TCF_MASK	(3UL << PR_MTE_TCF_SHIFT)
+# define PR_MTE_TAG_SHIFT	3
+# define PR_MTE_TAG_MASK	(0xffffUL << PR_MTE_TAG_SHIFT)
+#endif
+
 __BEGIN_DECLS
 
 /* Control process execution.  */

Re: [PATCH v3 6/8] linux: Add compatibility definitions to sys/prctl.h for MTE

[Siddhesh Poyarekar]

On 11/23/20 9:12 PM, Richard Earnshaw via Libc-alpha wrote:
> 
> Older versions of the Linux kernel headers obviously lack support for
> memory tagging, but we still want to be able to build in support when
> using those (obviously it can't be enabled on such systems).
> 
> The linux kernel extensions are made to the platform-independent
> header (linux/prctl.h), so this patch takes a similar approach.
> ---
>   sysdeps/unix/sysv/linux/sys/prctl.h | 18 ++++++++++++++++++
>   1 file changed, 18 insertions(+)
> 

The way x86_64 CET support does this is to add an architecture-specific 
prctl.h that includes the next prctl.h and adds its own definitions 
under it, thus keeping these changes separate.  It may be more 
consistent to stay with that approach within glibc.

Siddhesh

[PATCH v3 7/8] aarch64: Add sysv specific enabling code for memory tagging

[Richard Earnshaw]

[-- Attachment #1: Type: text/plain, Size: 856 bytes --]


Add various defines and stubs for enabling MTE on AArch64 sysv-like
systems such as Linux.  The HWCAP feature bit is copied over in the
same way as other feature bits.  Similarly we add a new wrapper header
for mman.h to define the PROT_MTE flag that can be used with mmap and
related functions.

We add a new field to struct cpu_features that can be used, for
example, to check whether or not certain ifunc'd routines should be
bound to MTE-safe versions.

Finally, if we detect that MTE should be enabled (ie via the glibc
tunable); we enable MTE during startup as required.
---
 sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h  |  1 +
 sysdeps/unix/sysv/linux/aarch64/bits/mman.h   |  7 +++++
 .../unix/sysv/linux/aarch64/cpu-features.c    | 28 +++++++++++++++++++
 .../unix/sysv/linux/aarch64/cpu-features.h    |  1 +
 4 files changed, 37 insertions(+)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v3-0007-aarch64-Add-sysv-specific-enabling-code-for-memor.patch --]
[-- Type: text/x-patch; name="v3-0007-aarch64-Add-sysv-specific-enabling-code-for-memor.patch", Size: 3219 bytes --]

diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h b/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h
index af90d8a626..389852f1d9 100644
--- a/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h
+++ b/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h
@@ -73,3 +73,4 @@
 #define HWCAP2_DGH		(1 << 15)
 #define HWCAP2_RNG		(1 << 16)
 #define HWCAP2_BTI		(1 << 17)
+#define HWCAP2_MTE		(1 << 18)
diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/mman.h b/sysdeps/unix/sysv/linux/aarch64/bits/mman.h
index ecae046344..3658b958b5 100644
--- a/sysdeps/unix/sysv/linux/aarch64/bits/mman.h
+++ b/sysdeps/unix/sysv/linux/aarch64/bits/mman.h
@@ -1,4 +1,5 @@
 /* Definitions for POSIX memory map interface.  Linux/AArch64 version.
+
    Copyright (C) 2020 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
 
@@ -25,6 +26,12 @@
 
 #define PROT_BTI	0x10
 
+/* The following definitions basically come from the kernel headers.
+   But the kernel header is not namespace clean.  */
+
+/* Other flags.  */
+#define PROT_MTE	0x20		/* Normal Tagged mapping.  */
+
 #include <bits/mman-map-flags-generic.h>
 
 /* Include generic Linux declarations.  */
diff --git a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
index b9ab827aca..aa4a82c6e8 100644
--- a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
+++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
@@ -19,6 +19,7 @@
 #include <cpu-features.h>
 #include <sys/auxv.h>
 #include <elf/dl-hwcaps.h>
+#include <sys/prctl.h>
 
 #define DCZID_DZP_MASK (1 << 4)
 #define DCZID_BS_MASK (0xf)
@@ -86,4 +87,31 @@ init_cpu_features (struct cpu_features *cpu_features)
 
   /* Check if BTI is supported.  */
   cpu_features->bti = GLRO (dl_hwcap2) & HWCAP2_BTI;
+
+  /* Setup memory tagging support if the HW and kernel support it, and if
+     the user has requested it.  */
+  cpu_features->mte_state = 0;
+
+#ifdef _LIBC_MTAG
+# if HAVE_TUNABLES
+  int mte_state = TUNABLE_GET (glibc, memtag, enable, unsigned, 0);
+  cpu_features->mte_state = (GLRO (dl_hwcap2) & HWCAP2_MTE) ? mte_state : 0;
+  /* If we lack the MTE feature, disable the tunable, since it will
+     otherwise cause instructions that won't run on this CPU to be used.  */
+  TUNABLE_SET (glibc, memtag, enable, unsigned, cpu_features->mte_state);
+# endif
+
+  /* For now, disallow tag 0, so that we can clearly see when tagged
+     addresses are being allocated.  */
+  if (cpu_features->mte_state & 2)
+    __prctl (PR_SET_TAGGED_ADDR_CTRL,
+	     (PR_TAGGED_ADDR_ENABLE | PR_MTE_TCF_SYNC
+	      | (0xfffe << PR_MTE_TAG_SHIFT)),
+	     0, 0, 0);
+  else if (cpu_features->mte_state)
+    __prctl (PR_SET_TAGGED_ADDR_CTRL,
+	     (PR_TAGGED_ADDR_ENABLE | PR_MTE_TCF_ASYNC
+	      | (0xfffe << PR_MTE_TAG_SHIFT)),
+	     0, 0, 0);
+#endif
 }
diff --git a/sysdeps/unix/sysv/linux/aarch64/cpu-features.h b/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
index 00a4d0c8e7..838d5c9aba 100644
--- a/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
+++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
@@ -70,6 +70,7 @@ struct cpu_features
   uint64_t midr_el1;
   unsigned zva_size;
   bool bti;
+  unsigned mte_state;
 };
 
 #endif /* _CPU_FEATURES_AARCH64_H  */

Re: [PATCH v3 7/8] aarch64: Add sysv specific enabling code for memory tagging

[Szabolcs Nagy]

The 11/23/2020 15:42, Richard Earnshaw wrote:
> --- a/sysdeps/unix/sysv/linux/aarch64/bits/mman.h
> +++ b/sysdeps/unix/sysv/linux/aarch64/bits/mman.h
> @@ -1,4 +1,5 @@
>  /* Definitions for POSIX memory map interface.  Linux/AArch64 version.
> +
>     Copyright (C) 2020 Free Software Foundation, Inc.
>     This file is part of the GNU C Library.
>  
> @@ -25,6 +26,12 @@
>  
>  #define PROT_BTI	0x10
>  
> +/* The following definitions basically come from the kernel headers.
> +   But the kernel header is not namespace clean.  */
> +
> +/* Other flags.  */
> +#define PROT_MTE	0x20		/* Normal Tagged mapping.  */
> +

the comments are not needed and the PROT_MTE one in
particular adds unnecessary risk: copying code from
linux uapi headers is ok (it's the api), but copying
comments is not ok (they are copyrightable).

i think all we need is a reference to the appropriate
linux uapi header which is already there for PROT_BTI.

Re: [PATCH v3 7/8] aarch64: Add sysv specific enabling code for memory tagging

[Richard Earnshaw (lists)]

On 23/11/2020 16:53, Szabolcs Nagy wrote:
> The 11/23/2020 15:42, Richard Earnshaw wrote:
>> --- a/sysdeps/unix/sysv/linux/aarch64/bits/mman.h
>> +++ b/sysdeps/unix/sysv/linux/aarch64/bits/mman.h
>> @@ -1,4 +1,5 @@
>>  /* Definitions for POSIX memory map interface.  Linux/AArch64 version.
>> +
>>     Copyright (C) 2020 Free Software Foundation, Inc.
>>     This file is part of the GNU C Library.
>>  
>> @@ -25,6 +26,12 @@
>>  
>>  #define PROT_BTI     0x10
>>  
>> +/* The following definitions basically come from the kernel headers.
>> +   But the kernel header is not namespace clean.  */
>> +
>> +/* Other flags.  */
>> +#define PROT_MTE     0x20            /* Normal Tagged mapping.  */
>> +
> 
> the comments are not needed and the PROT_MTE one in
> particular adds unnecessary risk: copying code from
> linux uapi headers is ok (it's the api), but copying
> comments is not ok (they are copyrightable).
> 
> i think all we need is a reference to the appropriate
> linux uapi header which is already there for PROT_BTI.

OK, will fix.

R.

Re: [PATCH v3 7/8] aarch64: Add sysv specific enabling code for memory tagging

[Siddhesh Poyarekar]

On 11/23/20 9:12 PM, Richard Earnshaw via Libc-alpha wrote:
> 
> Add various defines and stubs for enabling MTE on AArch64 sysv-like
> systems such as Linux.  The HWCAP feature bit is copied over in the
> same way as other feature bits.  Similarly we add a new wrapper header
> for mman.h to define the PROT_MTE flag that can be used with mmap and
> related functions.
> 
> We add a new field to struct cpu_features that can be used, for
> example, to check whether or not certain ifunc'd routines should be
> bound to MTE-safe versions.
> 
> Finally, if we detect that MTE should be enabled (ie via the glibc
> tunable); we enable MTE during startup as required.
> ---
>   sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h  |  1 +
>   sysdeps/unix/sysv/linux/aarch64/bits/mman.h   |  7 +++++
>   .../unix/sysv/linux/aarch64/cpu-features.c    | 28 +++++++++++++++++++
>   .../unix/sysv/linux/aarch64/cpu-features.h    |  1 +
>   4 files changed, 37 insertions(+)
> 
> diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h b/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h
> index af90d8a626..389852f1d9 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h
> +++ b/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h
> @@ -73,3 +73,4 @@
>  #define HWCAP2_DGH		(1 << 15)
>  #define HWCAP2_RNG		(1 << 16)
>  #define HWCAP2_BTI		(1 << 17)
> +#define HWCAP2_MTE		(1 << 18)
> diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/mman.h b/sysdeps/unix/sysv/linux/aarch64/bits/mman.h
> index ecae046344..3658b958b5 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/bits/mman.h
> +++ b/sysdeps/unix/sysv/linux/aarch64/bits/mman.h
> @@ -1,4 +1,5 @@
>  /* Definitions for POSIX memory map interface.  Linux/AArch64 version.
> +

Unnecessary newline.  I spotted a couple in 3/8 as well.

>     Copyright (C) 2020 Free Software Foundation, Inc.
>     This file is part of the GNU C Library.
>  
> @@ -25,6 +26,12 @@
>  
>  #define PROT_BTI	0x10
>  
> +/* The following definitions basically come from the kernel headers.
> +   But the kernel header is not namespace clean.  */
> +
> +/* Other flags.  */
> +#define PROT_MTE	0x20		/* Normal Tagged mapping.  */
> +
>  #include <bits/mman-map-flags-generic.h>
>  
>  /* Include generic Linux declarations.  */
> diff --git a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
> index b9ab827aca..aa4a82c6e8 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
> +++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
> @@ -19,6 +19,7 @@
>  #include <cpu-features.h>
>  #include <sys/auxv.h>
>  #include <elf/dl-hwcaps.h>
> +#include <sys/prctl.h>
>  
>  #define DCZID_DZP_MASK (1 << 4)
>  #define DCZID_BS_MASK (0xf)
> @@ -86,4 +87,31 @@ init_cpu_features (struct cpu_features *cpu_features)
>  
>    /* Check if BTI is supported.  */
>    cpu_features->bti = GLRO (dl_hwcap2) & HWCAP2_BTI;
> +
> +  /* Setup memory tagging support if the HW and kernel support it, and if
> +     the user has requested it.  */
> +  cpu_features->mte_state = 0;
> +
> +#ifdef _LIBC_MTAG
> +# if HAVE_TUNABLES
> +  int mte_state = TUNABLE_GET (glibc, memtag, enable, unsigned, 0);
> +  cpu_features->mte_state = (GLRO (dl_hwcap2) & HWCAP2_MTE) ? mte_state : 0;
> +  /* If we lack the MTE feature, disable the tunable, since it will
> +     otherwise cause instructions that won't run on this CPU to be used.  */
> +  TUNABLE_SET (glibc, memtag, enable, unsigned, cpu_features->mte_state);
> +# endif
> +
> +  /* For now, disallow tag 0, so that we can clearly see when tagged
> +     addresses are being allocated.  */
> +  if (cpu_features->mte_state & 2)
> +    __prctl (PR_SET_TAGGED_ADDR_CTRL,
> +	     (PR_TAGGED_ADDR_ENABLE | PR_MTE_TCF_SYNC
> +	      | (0xfffe << PR_MTE_TAG_SHIFT)),

Couldn't this magic number also become a macro too?

> +	     0, 0, 0);
> +  else if (cpu_features->mte_state)
> +    __prctl (PR_SET_TAGGED_ADDR_CTRL,
> +	     (PR_TAGGED_ADDR_ENABLE | PR_MTE_TCF_ASYNC
> +	      | (0xfffe << PR_MTE_TAG_SHIFT)),

Likewise.

> +	     0, 0, 0);
> +#endif
>  }
> diff --git a/sysdeps/unix/sysv/linux/aarch64/cpu-features.h b/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
> index 00a4d0c8e7..838d5c9aba 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
> +++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
> @@ -70,6 +70,7 @@ struct cpu_features
>    uint64_t midr_el1;
>    unsigned zva_size;
>    bool bti;
> +  unsigned mte_state;

This could be just a byte unless you foresee expanding the MTE flags 
beyond the 8 bits you've specified in the tunables.

>  };
>  
>  #endif /* _CPU_FEATURES_AARCH64_H  */

Re: [PATCH v3 7/8] aarch64: Add sysv specific enabling code for memory tagging

[Richard Earnshaw]

On 25/11/2020 15:34, Siddhesh Poyarekar wrote:
> On 11/23/20 9:12 PM, Richard Earnshaw via Libc-alpha wrote:
>>
>> Add various defines and stubs for enabling MTE on AArch64 sysv-like
>> systems such as Linux.  The HWCAP feature bit is copied over in the
>> same way as other feature bits.  Similarly we add a new wrapper header
>> for mman.h to define the PROT_MTE flag that can be used with mmap and
>> related functions.
>>
>> We add a new field to struct cpu_features that can be used, for
>> example, to check whether or not certain ifunc'd routines should be
>> bound to MTE-safe versions.
>>
>> Finally, if we detect that MTE should be enabled (ie via the glibc
>> tunable); we enable MTE during startup as required.
>> ---
>>   sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h  |  1 +
>>   sysdeps/unix/sysv/linux/aarch64/bits/mman.h   |  7 +++++
>>   .../unix/sysv/linux/aarch64/cpu-features.c    | 28 +++++++++++++++++++
>>   .../unix/sysv/linux/aarch64/cpu-features.h    |  1 +
>>   4 files changed, 37 insertions(+)
>>
>> diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h
>> b/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h
>> index af90d8a626..389852f1d9 100644
>> --- a/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h
>> +++ b/sysdeps/unix/sysv/linux/aarch64/bits/hwcap.h
>> @@ -73,3 +73,4 @@
>>  #define HWCAP2_DGH        (1 << 15)
>>  #define HWCAP2_RNG        (1 << 16)
>>  #define HWCAP2_BTI        (1 << 17)
>> +#define HWCAP2_MTE        (1 << 18)
>> diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/mman.h
>> b/sysdeps/unix/sysv/linux/aarch64/bits/mman.h
>> index ecae046344..3658b958b5 100644
>> --- a/sysdeps/unix/sysv/linux/aarch64/bits/mman.h
>> +++ b/sysdeps/unix/sysv/linux/aarch64/bits/mman.h
>> @@ -1,4 +1,5 @@
>>  /* Definitions for POSIX memory map interface.  Linux/AArch64 version.
>> +
> 
> Unnecessary newline.  I spotted a couple in 3/8 as well.
> 
>>     Copyright (C) 2020 Free Software Foundation, Inc.
>>     This file is part of the GNU C Library.
>>  
>> @@ -25,6 +26,12 @@
>>  
>>  #define PROT_BTI    0x10
>>  
>> +/* The following definitions basically come from the kernel headers.
>> +   But the kernel header is not namespace clean.  */
>> +
>> +/* Other flags.  */
>> +#define PROT_MTE    0x20        /* Normal Tagged mapping.  */
>> +
>>  #include <bits/mman-map-flags-generic.h>
>>  
>>  /* Include generic Linux declarations.  */
>> diff --git a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
>> b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
>> index b9ab827aca..aa4a82c6e8 100644
>> --- a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
>> +++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
>> @@ -19,6 +19,7 @@
>>  #include <cpu-features.h>
>>  #include <sys/auxv.h>
>>  #include <elf/dl-hwcaps.h>
>> +#include <sys/prctl.h>
>>  
>>  #define DCZID_DZP_MASK (1 << 4)
>>  #define DCZID_BS_MASK (0xf)
>> @@ -86,4 +87,31 @@ init_cpu_features (struct cpu_features *cpu_features)
>>  
>>    /* Check if BTI is supported.  */
>>    cpu_features->bti = GLRO (dl_hwcap2) & HWCAP2_BTI;
>> +
>> +  /* Setup memory tagging support if the HW and kernel support it,
>> and if
>> +     the user has requested it.  */
>> +  cpu_features->mte_state = 0;
>> +
>> +#ifdef _LIBC_MTAG
>> +# if HAVE_TUNABLES
>> +  int mte_state = TUNABLE_GET (glibc, memtag, enable, unsigned, 0);
>> +  cpu_features->mte_state = (GLRO (dl_hwcap2) & HWCAP2_MTE) ?
>> mte_state : 0;
>> +  /* If we lack the MTE feature, disable the tunable, since it will
>> +     otherwise cause instructions that won't run on this CPU to be
>> used.  */
>> +  TUNABLE_SET (glibc, memtag, enable, unsigned,
>> cpu_features->mte_state);
>> +# endif
>> +
>> +  /* For now, disallow tag 0, so that we can clearly see when tagged
>> +     addresses are being allocated.  */
>> +  if (cpu_features->mte_state & 2)
>> +    __prctl (PR_SET_TAGGED_ADDR_CTRL,
>> +         (PR_TAGGED_ADDR_ENABLE | PR_MTE_TCF_SYNC
>> +          | (0xfffe << PR_MTE_TAG_SHIFT)),
> 
> Couldn't this magic number also become a macro too?

Well, I could rewrite (0xfffe << PR_MTE_TAG_SHIFT) as

  (PR_MTE_TAG_MASK & ~(1UL << PR_MTE_TAG_SHIFT))

but I'm not entirely convinced that's a lot more understandable: it's
just a bit-mask of tags that can be enabled.

> 
>> +         0, 0, 0);
>> +  else if (cpu_features->mte_state)
>> +    __prctl (PR_SET_TAGGED_ADDR_CTRL,
>> +         (PR_TAGGED_ADDR_ENABLE | PR_MTE_TCF_ASYNC
>> +          | (0xfffe << PR_MTE_TAG_SHIFT)),
> 
> Likewise.
> 
>> +         0, 0, 0);
>> +#endif
>>  }
>> diff --git a/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
>> b/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
>> index 00a4d0c8e7..838d5c9aba 100644
>> --- a/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
>> +++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
>> @@ -70,6 +70,7 @@ struct cpu_features
>>    uint64_t midr_el1;
>>    unsigned zva_size;
>>    bool bti;
>> +  unsigned mte_state;
> 
> This could be just a byte unless you foresee expanding the MTE flags
> beyond the 8 bits you've specified in the tunables.

I don't know how stable this features structure has to be.  If it has to
remain fixed in layout forever (append-only?), then I think I'd rather
keep it as an unsigned to allow for more uses of the tunable as the need
arises.  If it's acceptable to just change the layout aribitrarily
between glibc releases, then using a byte for now would be fine.

> 
>>  };
>>  
>>  #endif /* _CPU_FEATURES_AARCH64_H  */
> 
> 

R.

Re: [PATCH v3 7/8] aarch64: Add sysv specific enabling code for memory tagging

[Siddhesh Poyarekar]

On 11/25/20 9:36 PM, Richard Earnshaw wrote:
> Well, I could rewrite (0xfffe << PR_MTE_TAG_SHIFT) as
> 
>    (PR_MTE_TAG_MASK & ~(1UL << PR_MTE_TAG_SHIFT))
> 
> but I'm not entirely convinced that's a lot more understandable: it's
> just a bit-mask of tags that can be enabled.

It isn't, I think what would work is a single macro that describes what 
(PR_MTE_TAG_MASK & ~(1UL << PR_MTE_TAG_SHIFT)) is, e.g.  DISALLOW_TAG_0 
assuming that that's the intention since I can't tell what it's for at 
the moment.

> I don't know how stable this features structure has to be.  If it has to
> remain fixed in layout forever (append-only?), then I think I'd rather
> keep it as an unsigned to allow for more uses of the tunable as the need
> arises.  If it's acceptable to just change the layout aribitrarily
> between glibc releases, then using a byte for now would be fine.

It's an internal structure, so we could do whatever we want with it.

Siddhesh

Re: [PATCH v3 7/8] aarch64: Add sysv specific enabling code for memory tagging

[Siddhesh Poyarekar]

On 11/25/20 9:50 PM, Siddhesh Poyarekar wrote:
> On 11/25/20 9:36 PM, Richard Earnshaw wrote:
>> Well, I could rewrite (0xfffe << PR_MTE_TAG_SHIFT) as
>>
>>    (PR_MTE_TAG_MASK & ~(1UL << PR_MTE_TAG_SHIFT))
>>
>> but I'm not entirely convinced that's a lot more understandable: it's
>> just a bit-mask of tags that can be enabled.
> 
> It isn't, I think what would work is a single macro that describes what 
> (PR_MTE_TAG_MASK & ~(1UL << PR_MTE_TAG_SHIFT)) is, e.g.  DISALLOW_TAG_0 
> assuming that that's the intention since I can't tell what it's for at 
> the moment.

Of course, I should have read your response properly before suggesting; 
ALLOWED_TAGS_BITMASK should be intuitive enough as long as one isn't 
talking to their kid while reading code.

Siddhesh

[PATCH v3 8/8] aarch64: Add aarch64-specific files for memory tagging support

[Richard Earnshaw]

[-- Attachment #1: Type: text/plain, Size: 813 bytes --]


This final patch provides the architecture-specific implementation of
the memory-tagging support hooks for aarch64.
---
 sysdeps/aarch64/Makefile                 |  5 +++
 sysdeps/aarch64/__mtag_address_get_tag.S | 31 +++++++++++++
 sysdeps/aarch64/__mtag_memset_tag.S      | 46 +++++++++++++++++++
 sysdeps/aarch64/__mtag_new_tag.S         | 38 ++++++++++++++++
 sysdeps/aarch64/__mtag_tag_region.S      | 44 ++++++++++++++++++
 sysdeps/aarch64/libc-mtag.h              | 57 ++++++++++++++++++++++++
 6 files changed, 221 insertions(+)
 create mode 100644 sysdeps/aarch64/__mtag_address_get_tag.S
 create mode 100644 sysdeps/aarch64/__mtag_memset_tag.S
 create mode 100644 sysdeps/aarch64/__mtag_new_tag.S
 create mode 100644 sysdeps/aarch64/__mtag_tag_region.S
 create mode 100644 sysdeps/aarch64/libc-mtag.h


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v3-0008-aarch64-Add-aarch64-specific-files-for-memory-tag.patch --]
[-- Type: text/x-patch; name="v3-0008-aarch64-Add-aarch64-specific-files-for-memory-tag.patch", Size: 8964 bytes --]

diff --git a/sysdeps/aarch64/Makefile b/sysdeps/aarch64/Makefile
index d8e06d27b2..2e88fc84a4 100644
--- a/sysdeps/aarch64/Makefile
+++ b/sysdeps/aarch64/Makefile
@@ -40,4 +40,9 @@ endif
 
 ifeq ($(subdir),misc)
 sysdep_headers += sys/ifunc.h
+sysdep_routines += __mtag_tag_region __mtag_new_tag __mtag_address_get_tag
+endif
+
+ifeq ($(subdir),string)
+sysdep_routines += __mtag_memset_tag
 endif
diff --git a/sysdeps/aarch64/__mtag_address_get_tag.S b/sysdeps/aarch64/__mtag_address_get_tag.S
new file mode 100644
index 0000000000..654c9d660c
--- /dev/null
+++ b/sysdeps/aarch64/__mtag_address_get_tag.S
@@ -0,0 +1,31 @@
+/* Copyright (C) 2020 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/>.  */
+
+#include <sysdep.h>
+
+#define ptr x0
+
+	.arch armv8.5-a
+	.arch_extension memtag
+
+ENTRY (__libc_mtag_address_get_tag)
+
+	ldg	ptr, [ptr]
+	ret
+END (__libc_mtag_address_get_tag)
+libc_hidden_builtin_def (__libc_mtag_address_get_tag)
diff --git a/sysdeps/aarch64/__mtag_memset_tag.S b/sysdeps/aarch64/__mtag_memset_tag.S
new file mode 100644
index 0000000000..bc98dc49d2
--- /dev/null
+++ b/sysdeps/aarch64/__mtag_memset_tag.S
@@ -0,0 +1,46 @@
+/* Copyright (C) 2020 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/>.  */
+
+#include <sysdep.h>
+/* Use the same register names and assignments as memset.  */
+#include "memset-reg.h"
+
+	.arch armv8.5-a
+	.arch_extension memtag
+
+/* NB, only supported on variants with 64-bit pointers.  */
+
+/* FIXME: This is a minimal implementation.  We could do much better than
+   this for large values of COUNT.  */
+
+ENTRY_ALIGN(__libc_mtag_memset_with_tag, 6)
+
+	and	valw, valw, 255
+	orr	valw, valw, valw, lsl 8
+	orr	valw, valw, valw, lsl 16
+	orr	val, val, val, lsl 32
+	mov	dst, dstin
+
+L(loop):
+	stgp	val, val, [dst], #16
+	subs	count, count, 16
+	bne	L(loop)
+	ldg	dstin, [dstin] // Recover the tag created (might be untagged).
+	ret
+END (__libc_mtag_memset_with_tag)
+libc_hidden_builtin_def (__libc_mtag_memset_with_tag)
diff --git a/sysdeps/aarch64/__mtag_new_tag.S b/sysdeps/aarch64/__mtag_new_tag.S
new file mode 100644
index 0000000000..3a22995e9f
--- /dev/null
+++ b/sysdeps/aarch64/__mtag_new_tag.S
@@ -0,0 +1,38 @@
+/* Copyright (C) 2020 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/>.  */
+
+#include <sysdep.h>
+
+	.arch armv8.5-a
+	.arch_extension memtag
+
+/* NB, only supported on variants with 64-bit pointers.  */
+
+/* FIXME: This is a minimal implementation.  We could do better than
+   this for larger values of COUNT.  */
+
+#define ptr x0
+#define xset x1
+
+ENTRY(__libc_mtag_new_tag)
+	// Guarantee that the new tag is not the same as now.
+	gmi	xset, ptr, xzr
+	irg	ptr, ptr, xset
+	ret
+END (__libc_mtag_new_tag)
+libc_hidden_builtin_def (__libc_mtag_new_tag)
diff --git a/sysdeps/aarch64/__mtag_tag_region.S b/sysdeps/aarch64/__mtag_tag_region.S
new file mode 100644
index 0000000000..41019781d0
--- /dev/null
+++ b/sysdeps/aarch64/__mtag_tag_region.S
@@ -0,0 +1,44 @@
+/* Copyright (C) 2020 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/>.  */
+
+#include <sysdep.h>
+/* Use the same register names and assignments as memset.  */
+
+	.arch armv8.5-a
+	.arch_extension memtag
+
+/* NB, only supported on variants with 64-bit pointers.  */
+
+/* FIXME: This is a minimal implementation.  We could do better than
+   this for larger values of COUNT.  */
+
+#define dstin x0
+#define count x1
+#define dst   x2
+
+ENTRY_ALIGN(__libc_mtag_tag_region, 6)
+
+	mov	dst, dstin
+L(loop):
+	stg	dst, [dst], #16
+	subs	count, count, 16
+	bne	L(loop)
+	ldg	dstin, [dstin] // Recover the tag created (might be untagged).
+	ret
+END (__libc_mtag_tag_region)
+libc_hidden_builtin_def (__libc_mtag_tag_region)
diff --git a/sysdeps/aarch64/libc-mtag.h b/sysdeps/aarch64/libc-mtag.h
new file mode 100644
index 0000000000..9c7d00c541
--- /dev/null
+++ b/sysdeps/aarch64/libc-mtag.h
@@ -0,0 +1,57 @@
+/* libc-internal interface for tagged (colored) memory support.
+   Copyright (C) 2020 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 _AARCH64_LIBC_MTAG_H
+#define _AARCH64_LIBC_MTAG_H 1
+
+#ifndef _LIBC_MTAG
+/* Generic bindings for systems that do not support memory tagging.  */
+#include_next "libc-mtag.h"
+#else
+
+/* Used to ensure additional alignment when objects need to have distinct
+   tags.  */
+#define __MTAG_GRANULE_SIZE 16
+
+/* Non-zero if memory obtained via morecore (sbrk) is not tagged.  */
+#define __MTAG_SBRK_UNTAGGED 1
+
+/* Extra flags to pass to mmap to get tagged pages.  */
+#define __MTAG_MMAP_FLAGS PROT_MTE
+
+/* Set the tags for a region of memory, which must have size and alignment
+   that are multiples of __MTAG_GRANULE_SIZE.  Size cannot be zero.
+   void *__libc_mtag_tag_region (const void *, size_t)  */
+void *__libc_mtag_tag_region (void *, size_t);
+
+/* Optimized equivalent to __libc_mtag_tag_region followed by memset.  */
+void *__libc_mtag_memset_with_tag(void *, int, size_t);
+
+/* Convert address P to a pointer that is tagged correctly for that
+   location.
+   void *__libc_mtag_address_get_tag (void*)  */
+void *__libc_mtag_address_get_tag(void *);
+
+/* Assign a new (random) tag to a pointer P (does not adjust the tag on
+   the memory addressed).
+   void *__libc_mtag_new_tag (void*)  */
+void *__libc_mtag_new_tag(void *);
+
+#endif /* _LIBC_MTAG */
+
+#endif /* _AARCH64_LIBC_MTAG_H */

Re: [PATCH v3 8/8] aarch64: Add aarch64-specific files for memory tagging support

[Szabolcs Nagy]

The 11/23/2020 15:42, Richard Earnshaw wrote:
> This final patch provides the architecture-specific implementation of
> the memory-tagging support hooks for aarch64.
...
> diff --git a/sysdeps/aarch64/Makefile b/sysdeps/aarch64/Makefile
> index d8e06d27b2..2e88fc84a4 100644
> --- a/sysdeps/aarch64/Makefile
> +++ b/sysdeps/aarch64/Makefile
> @@ -40,4 +40,9 @@ endif
>  
>  ifeq ($(subdir),misc)
>  sysdep_headers += sys/ifunc.h
> +sysdep_routines += __mtag_tag_region __mtag_new_tag __mtag_address_get_tag
> +endif
> +
> +ifeq ($(subdir),string)
> +sysdep_routines += __mtag_memset_tag
>  endif

this is ok, but i think all can go to misc.

> diff --git a/sysdeps/aarch64/__mtag_address_get_tag.S b/sysdeps/aarch64/__mtag_address_get_tag.S
> new file mode 100644
> index 0000000000..654c9d660c
> --- /dev/null
> +++ b/sysdeps/aarch64/__mtag_address_get_tag.S
> @@ -0,0 +1,31 @@
> +/* Copyright (C) 2020 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/>.  */
> +
> +#include <sysdep.h>
> +

i'd like the content after sysdep.h to be in

  #ifdef _LIBC_MTAG

so we don't build unused armv8.5-a code into glibc
(which can cause trouble with old binutils).

> +#define ptr x0
> +
> +	.arch armv8.5-a
> +	.arch_extension memtag
> +
> +ENTRY (__libc_mtag_address_get_tag)
> +
> +	ldg	ptr, [ptr]
> +	ret
> +END (__libc_mtag_address_get_tag)
> +libc_hidden_builtin_def (__libc_mtag_address_get_tag)

i think we don't need libc_hidden_builtin_def here:
that is only necessary when we make this a public api
(by adding it to a Versions file) but then we need
other changes like libc_hidden_proto on declarations
as well. as long as these are only used from libc.so
no magic is needed.

same for the other functions.

> diff --git a/sysdeps/aarch64/__mtag_memset_tag.S b/sysdeps/aarch64/__mtag_memset_tag.S
> new file mode 100644
> index 0000000000..bc98dc49d2
> --- /dev/null
> +++ b/sysdeps/aarch64/__mtag_memset_tag.S
> @@ -0,0 +1,46 @@
...
> +#include <sysdep.h>
> +/* Use the same register names and assignments as memset.  */
> +#include "memset-reg.h"
> +
> +	.arch armv8.5-a
> +	.arch_extension memtag
> +
> +/* NB, only supported on variants with 64-bit pointers.  */
> +
> +/* FIXME: This is a minimal implementation.  We could do much better than
> +   this for large values of COUNT.  */
> +
> +ENTRY_ALIGN(__libc_mtag_memset_with_tag, 6)

this should be plain ENTRY
(which defaults to 2^6 alignment)

same for __libc_mtag_tag_region.

> +
> +	and	valw, valw, 255
> +	orr	valw, valw, valw, lsl 8
> +	orr	valw, valw, valw, lsl 16
> +	orr	val, val, val, lsl 32
> +	mov	dst, dstin
> +
> +L(loop):
> +	stgp	val, val, [dst], #16
> +	subs	count, count, 16
> +	bne	L(loop)
> +	ldg	dstin, [dstin] // Recover the tag created (might be untagged).

i think ldg is not needed: for now we only use
this on PROT_MTE enabled memory.

(calling the function with non-heap memory or
disabling mte on heap memory can be treated as
a bug. this decision can be changed later if
we want to expose public mtag api to user code:
in particular the unwinder for stack tagging
may want to use such api, but we don't need to
design that right now.)

same for __libc_mtag_tag_region.

> +	ret
> +END (__libc_mtag_memset_with_tag)
> +libc_hidden_builtin_def (__libc_mtag_memset_with_tag)
> diff --git a/sysdeps/aarch64/__mtag_new_tag.S b/sysdeps/aarch64/__mtag_new_tag.S
> new file mode 100644
> index 0000000000..3a22995e9f
> --- /dev/null
> +++ b/sysdeps/aarch64/__mtag_new_tag.S
> @@ -0,0 +1,38 @@
...
> +#include <sysdep.h>
> +
> +	.arch armv8.5-a
> +	.arch_extension memtag
> +
> +/* NB, only supported on variants with 64-bit pointers.  */
> +
> +/* FIXME: This is a minimal implementation.  We could do better than
> +   this for larger values of COUNT.  */

this fixme is not needed here.

> +
> +#define ptr x0
> +#define xset x1
> +
> +ENTRY(__libc_mtag_new_tag)
> +	// Guarantee that the new tag is not the same as now.
> +	gmi	xset, ptr, xzr
> +	irg	ptr, ptr, xset
> +	ret
> +END (__libc_mtag_new_tag)
> +libc_hidden_builtin_def (__libc_mtag_new_tag)
...

rest looked ok.

Re: [PATCH v3 0/8] Memory tagging support

[Szabolcs Nagy]

The 11/23/2020 15:42, Richard Earnshaw wrote:
> This is the third iteration of the patch set to enable memory tagging
> in glibc's malloc code.  It mainly addresses the following issues raised
> during the previous review:
> 
>  - Clean up/add some internal API documentation
>  - Remove ROUND_UP_ALLOCATION_SIZE and use some conditionalized code
>    in checked_request2size instead.
>  - Support MALLOC_CHECK_ in conjuction with _MTAG_ENABLE.
> 
> The first two issues are addressed in patch 4 of this series, and the
> third in patch 5.  I intend to merge patches 3, 4 and 5 into a single
> update to the malloc code before the final commit; I've kept them
> separate for now to (hopefully) simplify the review.
> 
> The patches have all been rebased against master as of 2020/11/20.
> 
> I spent quite a bit of time while working on these looking at whether
> the code could be refactored in order to reduce the places where
> SIZE_SZ was being added (in different multiples) to various pointers.
> I eventually concluded that this wasn't significantly improving the
> readability of the code, but one change has survived - I've replaced
> usage of 2 * SIZE_SZ with CHUNK_HDR_SZ when it is clear that this is
> referring to the header block at the start of a chunk.
> 
> I've pushed a copy of this patch series to rearnsha/mte-v3.0, since I
> understand some people want to try the patch series as a whole.

cross testing with --enable-memory-tagging and
_MTAG_ENABLE=3 in qemu with arm64 for-next/mte
linux branch, the new failures i see:

FAIL: malloc/tst-malloc-backtrace
FAIL: malloc/tst-mallocstate
FAIL: malloc/tst-safe-linking
FAIL: malloc/tst-tcfree1
FAIL: malloc/tst-tcfree2
FAIL: malloc/tst-tcfree3
	all these tests have use after free
FAIL: posix/tst-mmap
	unaligned mmap over malloced memory now
	fails with ENOMEM instead of EINVAL.

i think these are all reasonable failures for mte.
(the malloc/ ones can be unsupported, and the mmap
test can use mmaped memory instead of malloced)

Re: [PATCH v3 0/8] Memory tagging support

[Siddhesh Poyarekar]

On 11/23/20 9:12 PM, Richard Earnshaw via Libc-alpha wrote:
> This is the third iteration of the patch set to enable memory tagging
> in glibc's malloc code.  It mainly addresses the following issues raised
> during the previous review:
> 
>   - Clean up/add some internal API documentation
>   - Remove ROUND_UP_ALLOCATION_SIZE and use some conditionalized code
>     in checked_request2size instead.
>   - Support MALLOC_CHECK_ in conjuction with _MTAG_ENABLE.

I tried to review but the patchset doesn't build on x86_64 because 
__libc_mtag_tag_region() and __libc_mtag_address_get_tag() are macros:

arena.c: In function ‘ptmalloc_init’:
arena.c:342:22: error: ‘__libc_mtag_tag_region’ undeclared (first use in 
this function); did you mean ‘__default_tag_region’?
   342 |       __tag_region = __libc_mtag_tag_region;
       |                      ^~~~~~~~~~~~~~~~~~~~~~
       |                      __default_tag_region

I'll review 1, 2. 6 and 7 (and the overall design) anyway since I 
suspect you'll only end up changing 3 and 8 to fix this failure. 
Hopefully that will help you make forward progress.

> The first two issues are addressed in patch 4 of this series, and the
> third in patch 5.  I intend to merge patches 3, 4 and 5 into a single
> update to the malloc code before the final commit; I've kept them
> separate for now to (hopefully) simplify the review.

Please merge them in first; it is actually confusing to review because I 
have to then refer across three patches to see what's fixed in 3/8.

Besides, having the version of the patch in review being the same as the 
one that's committed helps since we can then auto-close patchwork patches.

> I spent quite a bit of time while working on these looking at whether
> the code could be refactored in order to reduce the places where
> SIZE_SZ was being added (in different multiples) to various pointers.
> I eventually concluded that this wasn't significantly improving the
> readability of the code, but one change has survived - I've replaced
> usage of 2 * SIZE_SZ with CHUNK_HDR_SZ when it is clear that this is
> referring to the header block at the start of a chunk.

I skimmed through the patches and I think this is a useful change, thanks.

Siddhesh

Re: [PATCH v3 0/8] Memory tagging support

[Richard Earnshaw]

On 25/11/2020 14:49, Siddhesh Poyarekar wrote:
> On 11/23/20 9:12 PM, Richard Earnshaw via Libc-alpha wrote:
>> This is the third iteration of the patch set to enable memory tagging
>> in glibc's malloc code.  It mainly addresses the following issues raised
>> during the previous review:
>>
>>   - Clean up/add some internal API documentation
>>   - Remove ROUND_UP_ALLOCATION_SIZE and use some conditionalized code
>>     in checked_request2size instead.
>>   - Support MALLOC_CHECK_ in conjuction with _MTAG_ENABLE.
> 
> I tried to review but the patchset doesn't build on x86_64 because
> __libc_mtag_tag_region() and __libc_mtag_address_get_tag() are macros:
> 
> arena.c: In function ‘ptmalloc_init’:
> arena.c:342:22: error: ‘__libc_mtag_tag_region’ undeclared (first use in
> this function); did you mean ‘__default_tag_region’?
>   342 |       __tag_region = __libc_mtag_tag_region;
>       |                      ^~~~~~~~~~~~~~~~~~~~~~
>       |                      __default_tag_region
> 

Ah, you're configuring x86 with this feature enabled.  Sorry, I didn't
think to test that :(

I think the right solution is to make the configure step detect that
this won't work and ignore the config option in that case.


> I'll review 1, 2. 6 and 7 (and the overall design) anyway since I
> suspect you'll only end up changing 3 and 8 to fix this failure.
> Hopefully that will help you make forward progress.
> 
>> The first two issues are addressed in patch 4 of this series, and the
>> third in patch 5.  I intend to merge patches 3, 4 and 5 into a single
>> update to the malloc code before the final commit; I've kept them
>> separate for now to (hopefully) simplify the review.
> 
> Please merge them in first; it is actually confusing to review because I
> have to then refer across three patches to see what's fixed in 3/8.
> 
> Besides, having the version of the patch in review being the same as the
> one that's committed helps since we can then auto-close patchwork patches.

OK.

> 
>> I spent quite a bit of time while working on these looking at whether
>> the code could be refactored in order to reduce the places where
>> SIZE_SZ was being added (in different multiples) to various pointers.
>> I eventually concluded that this wasn't significantly improving the
>> readability of the code, but one change has survived - I've replaced
>> usage of 2 * SIZE_SZ with CHUNK_HDR_SZ when it is clear that this is
>> referring to the header block at the start of a chunk.
> 
> I skimmed through the patches and I think this is a useful change, thanks.
> 
> Siddhesh

R.

Re: [PATCH v3 0/8] Memory tagging support

[Siddhesh Poyarekar]

On 11/25/20 9:18 PM, Richard Earnshaw wrote:
> Ah, you're configuring x86 with this feature enabled.  Sorry, I didn't
> think to test that :(
> 
> I think the right solution is to make the configure step detect that
> this won't work and ignore the config option in that case.
> 

If you take that route then I think the earlier description of 
--enable-memory-tagging in configure is more precise and you'd need to 
adjust the description in INSTALL ;)

Siddhesh

Re: [PATCH v3 0/8] Memory tagging support

[H.J. Lu]

On Mon, Nov 23, 2020 at 7:46 AM Richard Earnshaw via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> This is the third iteration of the patch set to enable memory tagging
> in glibc's malloc code.  It mainly addresses the following issues raised
> during the previous review:
>
>  - Clean up/add some internal API documentation
>  - Remove ROUND_UP_ALLOCATION_SIZE and use some conditionalized code
>    in checked_request2size instead.
>  - Support MALLOC_CHECK_ in conjuction with _MTAG_ENABLE.
>
> The first two issues are addressed in patch 4 of this series, and the
> third in patch 5.  I intend to merge patches 3, 4 and 5 into a single
> update to the malloc code before the final commit; I've kept them
> separate for now to (hopefully) simplify the review.
>
> The patches have all been rebased against master as of 2020/11/20.
>
> I spent quite a bit of time while working on these looking at whether
> the code could be refactored in order to reduce the places where
> SIZE_SZ was being added (in different multiples) to various pointers.
> I eventually concluded that this wasn't significantly improving the
> readability of the code, but one change has survived - I've replaced
> usage of 2 * SIZE_SZ with CHUNK_HDR_SZ when it is clear that this is
> referring to the header block at the start of a chunk.
>
> I've pushed a copy of this patch series to rearnsha/mte-v3.0, since I
> understand some people want to try the patch series as a whole.
>
> R.
>
> Richard Earnshaw (8):
>   config: Allow memory tagging to be enabled when configuring glibc
>   elf: Add a tunable to control use of tagged memory
>   malloc: Basic support for memory tagging in the malloc() family
>   malloc: Clean up commentary
>   malloc: support MALLOC_CHECK_ in conjunction with _MTAG_ENABLE.
>   linux: Add compatibility definitions to sys/prctl.h for MTE
>   aarch64: Add sysv specific enabling code for memory tagging
>   aarch64: Add aarch64-specific files for memory tagging support
>

Please add some tests to verify use-after-free and underflow/overflow detections
which aren't detected by the current malloc implementation.

Also should glibc provide a memory tag interface for other memory allocators?

H.J.

Re: [PATCH v3 0/8] Memory tagging support

[DJ Delorie]

So I gave the full patch set a review (having forgotten what I mentioned
the last time :-P ) keeping in mind my original comments when you first
brought this up - "As long as you don't impact other platforms, go for
it."  :-)

I did note that sometimes you used the "(void) ..." idiom and sometimes
you didn't, and as I just noted that in another thread, I feel obliged
to be consistent here, and ask that you use it consistently ;-)

But otherwise LGETM (looks good enough for me!) :-)

Reviewed-by: DJ Delorie <dj@redhat.com>

Re: [PATCH v3 0/8] Memory tagging support

[Richard Earnshaw]

On 17/12/2020 03:57, DJ Delorie via Libc-alpha wrote:
> 
> So I gave the full patch set a review (having forgotten what I mentioned
> the last time :-P ) keeping in mind my original comments when you first
> brought this up - "As long as you don't impact other platforms, go for
> it."  :-)
> 
> I did note that sometimes you used the "(void) ..." idiom and sometimes
> you didn't, and as I just noted that in another thread, I feel obliged
> to be consistent here, and ask that you use it consistently ;-)
> 

I really don't like the (void) idiom, but used it in a couple of cases
because it's needed.  For example,

+  /* Mark the chunk as belonging to the library again.  */
+  (void)TAG_REGION (chunk2rawmem (p), __malloc_usable_size (mem));

TAG_REGION returns it's first argument (because there are cases where
this valude is wanted) and in case where MTE is disabled, this just
expands to a statement with no effect - and thus a compiler warning.
The "(void)" idiom suppresses those warnings.


> But otherwise LGETM (looks good enough for me!) :-)
> 
> Reviewed-by: DJ Delorie <dj@redhat.com>
> 

Many thanks,

R.