[v7,1/2] arm64: Define Documentation/arm64/tagged-address-abi.rst

Commit Message

Catalin Marinas Aug. 7, 2019, 3:53 p.m. UTC

From: Vincenzo Frascino <[e-mail]>

On arm64 the TCR_EL1.TBI0 bit has been always enabled hence
the userspace (EL0) is allowed to set a non-zero value in the
top byte but the resulting pointers are not allowed at the
user-kernel syscall ABI boundary.

With the relaxed ABI proposed through this document, it is now possible
to pass tagged pointers to the syscalls, when these pointers are in
memory ranges obtained by an anonymous (MAP_ANONYMOUS) mmap().

This change in the ABI requires a mechanism to requires the userspace
to opt-in to such an option.

Specify and document the way in which sysctl and prctl() can be used
in combination to allow the userspace to opt-in this feature.

Cc: Will Deacon <[e-mail]>
Cc: Andrey Konovalov <[e-mail]>
Cc: Szabolcs Nagy <[e-mail]>
Cc: Kevin Brodsky <[e-mail]>
Signed-off-by: Vincenzo Frascino <[e-mail]>
[[e-mail]: some rewording, dropped MAP_PRIVATE]
Signed-off-by: Catalin Marinas <[e-mail]>
---
 Documentation/arm64/tagged-address-abi.rst | 151 +++++++++++++++++++++
 1 file changed, 151 insertions(+)
 create mode 100644 Documentation/arm64/tagged-address-abi.rst

Comments

Dave Hansen Aug. 7, 2019, 8:38 p.m. UTC | #1

On 8/7/19 8:53 AM, Catalin Marinas wrote:
> +- mmap() done by the process itself (or its parent), where either:
> +
> +  - flags have the **MAP_ANONYMOUS** bit set
> +  - the file descriptor refers to a regular file (including those returned
> +    by memfd_create()) or **/dev/zero**

What's a "regular file"? ;)

> +- brk() system call done by the process itself (i.e. the heap area between
> +  the initial location of the program break at process creation and its
> +  current location).
> +
> +- any memory mapped by the kernel in the address space of the process
> +  during creation and with the same restrictions as for mmap() above (e.g.
> +  data, bss, stack).
> +
> +The AArch64 Tagged Address ABI is an opt-in feature and an application can
> +control it via **prctl()** as follows:
> +
> +- **PR_SET_TAGGED_ADDR_CTRL**: enable or disable the AArch64 Tagged Address
> +  ABI for the calling process.
> +
> +  The (unsigned int) arg2 argument is a bit mask describing the control mode
> +  used:
> +
> +  - **PR_TAGGED_ADDR_ENABLE**: enable AArch64 Tagged Address ABI. Default
> +    status is disabled.
> +
> +  The arguments arg3, arg4, and arg5 are ignored.

For previous prctl()'s, we've found that it's best to require that the
unused arguments be 0.  Without that, apps are free to put garbage
there, which makes extending the prctl to use other arguments impossible
in the future.

Also, shouldn't this be converted over to an arch_prctl()?

> +The prctl(PR_SET_TAGGED_ADDR_CTRL, ...) will return -EINVAL if the
> +AArch64 Tagged Address ABI is not available
> +(CONFIG_ARM64_TAGGED_ADDR_ABI disabled or sysctl abi.tagged_addr=0).
> +
> +The ABI properties set by the mechanism described above are inherited by
> +threads of the same application and fork()'ed children but cleared by
> +execve().

What is the scope of these prctl()'s?  Are they thread-scoped or
process-scoped?  Can two threads in the same process run with different
tagging ABI modes?

> +Opting in (the prctl() option described above only) to or out of the
> +AArch64 Tagged Address ABI can be disabled globally at runtime using the
> +sysctl interface:
> +
> +- **abi.tagged_addr**: a new sysctl interface that can be used to prevent
> +  applications from enabling or disabling the relaxed ABI. The sysctl
> +  supports the following configuration options:
> +
> +  - **0**: disable the prctl(PR_SET_TAGGED_ADDR_CTRL) option to
> +    enable/disable the AArch64 Tagged Address ABI globally
> +
> +  - **1** (Default): enable the prctl(PR_SET_TAGGED_ADDR_CTRL) option to
> +    enable/disable the AArch64 Tagged Address ABI globally
> +
> +  Note that this sysctl does not affect the status of the AArch64 Tagged
> +  Address ABI of the running processes.

Shouldn't the name be "abi.tagged_addr_control" or something?  It
actually has *zero* direct effect on tagged addresses in the ABI.

What's the reason for allowing it to be toggled at runtime like this?
Wouldn't it make more sense to just have it be a boot option so you
*know* what the state of individual processes is?

> +When a process has successfully enabled the new ABI by invoking
> +prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE), the following
> +behaviours are guaranteed:
> +
> +- Every currently available syscall, except the cases mentioned in section
> +  3, can accept any valid tagged pointer. The same rule is applicable to
> +  any syscall introduced in the future.
> +
> +- The syscall behaviour is undefined for non valid tagged pointers.

Do you really mean "undefined"?  I mean, a bad pointer is a bad pointer.
 Why should it matter if it's a tagged bad pointer or an untagged bad
pointer?

...
> +A definition of the meaning of tagged pointers on AArch64 can be found in:
> +Documentation/arm64/tagged-pointers.txt.
> +
> +3. AArch64 Tagged Address ABI Exceptions
> +-----------------------------------------
> +
> +The behaviour described in section 2, with particular reference to the
> +acceptance by the syscalls of any valid tagged pointer, is not applicable
> +to the following cases:

This is saying things in a pretty roundabout manner.  Can't it just say:
 "The following cases do not accept tagged pointers:"

> +- mmap() addr parameter.
> +
> +- mremap() new_address parameter.

Is munmap() missing?  Or was there a reason for leaving it out?

> +- prctl(PR_SET_MM, ``*``, ...) other than arg2 PR_SET_MM_MAP and
> +  PR_SET_MM_MAP_SIZE.
> +
> +- prctl(PR_SET_MM, PR_SET_MM_MAP{,_SIZE}, ...) struct prctl_mm_map fields.
> +
> +Any attempt to use non-zero tagged pointers will lead to undefined
> +behaviour.

I wonder if you want to generalize this a bit.  I think you're saying
that parts of the ABI that modify the *layout* of the address space
never accept tagged pointers.

> +4. Example of correct usage
> +---------------------------
> +.. code-block:: c
> +
> +   void main(void)
> +   {
> +           static int tbi_enabled = 0;
> +           unsigned long tag = 0;
> +
> +           char *ptr = mmap(NULL, PAGE_SIZE, PROT_READ | PROT_WRITE,
> +                            MAP_ANONYMOUS, -1, 0);
> +
> +           if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE,
> +                     0, 0, 0) == 0)
> +                   tbi_enabled = 1;
> +
> +           if (ptr == (void *)-1) /* MAP_FAILED */
> +                   return -1;
> +
> +           if (tbi_enabled)
> +                   tag = rand() & 0xff;
> +
> +           ptr = (char *)((unsigned long)ptr | (tag << TAG_SHIFT));
> +
> +           *ptr = 'a';
> +
> +           ...
> +   }

It looks like the TAG_SHIFT and tag size are pretty baked into the
aarch64 architecture.  But, are you confident that no future
implementations will want different positions or sizes?  (obviously
controlled by other TCR_EL1 bits)

Szabolcs Nagy Aug. 8, 2019, 9:25 a.m. UTC | #2

On 07/08/2019 21:38, Dave Hansen wrote:
> On 8/7/19 8:53 AM, Catalin Marinas wrote:
>> +- The syscall behaviour is undefined for non valid tagged pointers.
> 
> Do you really mean "undefined"?  I mean, a bad pointer is a bad pointer.
>  Why should it matter if it's a tagged bad pointer or an untagged bad
> pointer?

bad pointers are invalid, but some non-bad pointers are
also invalid if they are tagged (e.g. tagged pointer to
device memory?) those may be valid to dereference in
userspace but don't work across the syscall abi (device
driver does not handle the tag?).

>> +- mmap() addr parameter.
>> +
>> +- mremap() new_address parameter.
> 
> Is munmap() missing?  Or was there a reason for leaving it out?

the new address in mmap and mremap may not be currently
mapped, other m* functions operate on existing mappings
(munmap, madvise, mprotect, mlock,...)

although by this logic brk (and related PR_SET_MM_*)
should be excluded here too.

>> +- prctl(PR_SET_MM, ``*``, ...) other than arg2 PR_SET_MM_MAP and
>> +  PR_SET_MM_MAP_SIZE.
>> +
>> +- prctl(PR_SET_MM, PR_SET_MM_MAP{,_SIZE}, ...) struct prctl_mm_map fields.
>> +
>> +Any attempt to use non-zero tagged pointers will lead to undefined
>> +behaviour.
> 
> I wonder if you want to generalize this a bit.  I think you're saying
> that parts of the ABI that modify the *layout* of the address space
> never accept tagged pointers.

something like that, but i think this is hard to specify
in a generic way.

Szabolcs Nagy Aug. 8, 2019, 4:20 p.m. UTC | #3

On 07/08/2019 21:38, Dave Hansen wrote:
> On 8/7/19 8:53 AM, Catalin Marinas wrote:
>> +- mmap() done by the process itself (or its parent), where either:
>> +
>> +  - flags have the **MAP_ANONYMOUS** bit set
>> +  - the file descriptor refers to a regular file (including those returned
>> +    by memfd_create()) or **/dev/zero**
> 
> What's a "regular file"? ;)

i'd expect the posix definition.

in posix "File types include regular file, character
special file, block special file, FIFO special file,
symbolic link, socket, and directory. Other types of
files may be supported by the implementation."

where regular file is "A file that is a randomly
accessible sequence of bytes, with no further
structure imposed by the system."
http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_323

Dave Martin Aug. 8, 2019, 4:27 p.m. UTC | #4

On Wed, Aug 07, 2019 at 01:38:16PM -0700, Dave Hansen wrote:

[Random comments below on a couple of points]

> On 8/7/19 8:53 AM, Catalin Marinas wrote:
> > +- mmap() done by the process itself (or its parent), where either:
> > +
> > +  - flags have the **MAP_ANONYMOUS** bit set
> > +  - the file descriptor refers to a regular file (including those returned
> > +    by memfd_create()) or **/dev/zero**
> 
> What's a "regular file"? ;)

A file, as distinct from device nodes, sockets, symlinks etc.

I think this is fairly standard UNIX terminology, even though it sounds
vague:

From glibc's <bits/stat.h>:

#define	__S_IFREG	0100000	/* Regular file.  */


Or for POSIX test (a.k.a. "[")

       -f file
              True if file exists and is a regular file.

Using memfd_create() or opening /dev/zero doesn't yield a regular file
though, so perhaps those should be a separate bullet.

[...]

> > +The AArch64 Tagged Address ABI is an opt-in feature and an application can
> > +control it via **prctl()** as follows:
> > +
> > +- **PR_SET_TAGGED_ADDR_CTRL**: enable or disable the AArch64 Tagged Address
> > +  ABI for the calling process.
> > +
> > +  The (unsigned int) arg2 argument is a bit mask describing the control mode
> > +  used:
> > +
> > +  - **PR_TAGGED_ADDR_ENABLE**: enable AArch64 Tagged Address ABI. Default
> > +    status is disabled.
> > +
> > +  The arguments arg3, arg4, and arg5 are ignored.
> 
> For previous prctl()'s, we've found that it's best to require that the
> unused arguments be 0.  Without that, apps are free to put garbage
> there, which makes extending the prctl to use other arguments impossible
> in the future.

Because arg2 is already a mask of flags with some flags unallocated,
we can add a new flag for ABI extensions.

If arg3 is used someday, it may or may not be natural for 0 to mean
"default".  Enabling this argument with an explicit flag in arg2 may
be cleaner than mangling the semantics of arg3 so that 0 can have
the right meaning.

Avoiding redundant 0 arguments also allows userspace to take advantage
of the glibc's variadic prototype for prctl() for example.

Not a huge deal, but that was my rationale anyway.

> Also, shouldn't this be converted over to an arch_prctl()?

Most arch-specific prctls seem to use prctl(), and arm64 already has a
few there.

arch_prctl() is x86-specific.  I don't know the history.

[...]

Cheers
---Dave

Szabolcs Nagy Aug. 8, 2019, 4:30 p.m. UTC | #5

On 07/08/2019 16:53, Catalin Marinas wrote:
> From: Vincenzo Frascino <[e-mail]>
> 
> On arm64 the TCR_EL1.TBI0 bit has been always enabled hence
> the userspace (EL0) is allowed to set a non-zero value in the
> top byte but the resulting pointers are not allowed at the
> user-kernel syscall ABI boundary.
> 
> With the relaxed ABI proposed through this document, it is now possible
> to pass tagged pointers to the syscalls, when these pointers are in
> memory ranges obtained by an anonymous (MAP_ANONYMOUS) mmap().

description needs to be updated not to restrict tags
to anon mmap.

> +3. AArch64 Tagged Address ABI Exceptions
> +-----------------------------------------
> +
> +The behaviour described in section 2, with particular reference to the
> +acceptance by the syscalls of any valid tagged pointer, is not applicable
> +to the following cases:
> +
> +- mmap() addr parameter.
> +
> +- mremap() new_address parameter.
> +
> +- prctl(PR_SET_MM, ``*``, ...) other than arg2 PR_SET_MM_MAP and
> +  PR_SET_MM_MAP_SIZE.
> +
> +- prctl(PR_SET_MM, PR_SET_MM_MAP{,_SIZE}, ...) struct prctl_mm_map fields.
> +
> +Any attempt to use non-zero tagged pointers will lead to undefined
> +behaviour.

i think that brk may be affected too by whatever that's
causing problems in mmap.

otherwise the text looks good to me.

Will Deacon Aug. 8, 2019, 5:04 p.m. UTC | #6

On Wed, Aug 07, 2019 at 04:53:20PM +0100, Catalin Marinas wrote:
> From: Vincenzo Frascino <[e-mail]>
> 
> On arm64 the TCR_EL1.TBI0 bit has been always enabled hence
> the userspace (EL0) is allowed to set a non-zero value in the
> top byte but the resulting pointers are not allowed at the
> user-kernel syscall ABI boundary.
> 
> With the relaxed ABI proposed through this document, it is now possible
> to pass tagged pointers to the syscalls, when these pointers are in
> memory ranges obtained by an anonymous (MAP_ANONYMOUS) mmap().
> 
> This change in the ABI requires a mechanism to requires the userspace
> to opt-in to such an option.
> 
> Specify and document the way in which sysctl and prctl() can be used
> in combination to allow the userspace to opt-in this feature.
> 
> Cc: Will Deacon <[e-mail]>
> Cc: Andrey Konovalov <[e-mail]>
> Cc: Szabolcs Nagy <[e-mail]>
> Cc: Kevin Brodsky <[e-mail]>
> Signed-off-by: Vincenzo Frascino <[e-mail]>
> [[e-mail]: some rewording, dropped MAP_PRIVATE]
> Signed-off-by: Catalin Marinas <[e-mail]>
> ---
>  Documentation/arm64/tagged-address-abi.rst | 151 +++++++++++++++++++++
>  1 file changed, 151 insertions(+)
>  create mode 100644 Documentation/arm64/tagged-address-abi.rst
> 
> diff --git a/Documentation/arm64/tagged-address-abi.rst b/Documentation/arm64/tagged-address-abi.rst
> new file mode 100644
> index 000000000000..f91a5d2ac865
> --- /dev/null
> +++ b/Documentation/arm64/tagged-address-abi.rst
> @@ -0,0 +1,151 @@
> +==========================
> +AArch64 TAGGED ADDRESS ABI
> +==========================
> +
> +Author: Vincenzo Frascino <[e-mail]>
> +
> +Date: 25 July 2019
> +
> +This document describes the usage and semantics of the Tagged Address
> +ABI on AArch64 Linux.
> +
> +1. Introduction
> +---------------
> +
> +On AArch64 the TCR_EL1.TBI0 bit has always been enabled, allowing userspace
> +(EL0) to perform memory accesses through 64-bit pointers with a non-zero
> +top byte. Such tagged pointers, however, were not allowed at the
> +user-kernel syscall ABI boundary.

I think we should drop the temporal language, so:

  "has always been enabled" => "is set by the kernel"
  "were not allowed" => "are not allowed by default"

> +
> +This document describes the relaxation of the syscall ABI that allows
> +userspace to pass certain tagged pointers to kernel syscalls, as described
> +in section 2.
> +
> +2. AArch64 Tagged Address ABI
> +-----------------------------
> +
> +From the kernel syscall interface perspective and for the purposes of this
> +document, a "valid tagged pointer" is a pointer with a potentially non-zero
> +top-byte that references an address in the user process address space
> +obtained in one of the following ways:
> +
> +- mmap() done by the process itself (or its parent), where either:
> +
> +  - flags have the **MAP_ANONYMOUS** bit set
> +  - the file descriptor refers to a regular file (including those returned
> +    by memfd_create()) or **/dev/zero**
> +
> +- brk() system call done by the process itself (i.e. the heap area between
> +  the initial location of the program break at process creation and its
> +  current location).
> +
> +- any memory mapped by the kernel in the address space of the process
> +  during creation and with the same restrictions as for mmap() above (e.g.
> +  data, bss, stack).
> +
> +The AArch64 Tagged Address ABI is an opt-in feature and an application can
> +control it via **prctl()** as follows:
> +
> +- **PR_SET_TAGGED_ADDR_CTRL**: enable or disable the AArch64 Tagged Address
> +  ABI for the calling process.
> +
> +  The (unsigned int) arg2 argument is a bit mask describing the control mode
> +  used:
> +
> +  - **PR_TAGGED_ADDR_ENABLE**: enable AArch64 Tagged Address ABI. Default
> +    status is disabled.
> +
> +  The arguments arg3, arg4, and arg5 are ignored.
> +
> +- **PR_GET_TAGGED_ADDR_CTRL**: get the status of the AArch64 Tagged Address
> +  ABI for the calling process.
> +
> +  The arguments arg2, arg3, arg4, and arg5 are ignored.

I agree with Dave (H) that we should require these to be zero. We may be
able to use arg2 to namespace things for PR_SET_TAGGED_ADDR_CTRL, but for
PR_GET_TAGGED_ADDR_CTRL we'd have to add a new prctl if we wanted to extend
it otherwise.

> +The prctl(PR_SET_TAGGED_ADDR_CTRL, ...) will return -EINVAL if the

*The* prctl? Maybe "Calling prctl(..." is better?

> +AArch64 Tagged Address ABI is not available
> +(CONFIG_ARM64_TAGGED_ADDR_ABI disabled or sysctl abi.tagged_addr=0).

drop the brackets and say "because CONFIG_... is disabled or ..".

> +
> +The ABI properties set by the mechanism described above are inherited by
> +threads of the same application and fork()'ed children but cleared by
> +execve().

Maybe just exec() here, since there are other flavours we shouldn't need to
enumerate.

> +Opting in (the prctl() option described above only) to or out of the
> +AArch64 Tagged Address ABI can be disabled globally at runtime using the
> +sysctl interface:

This sentence reads really badly thanks to the random bracketed part.

> +
> +- **abi.tagged_addr**: a new sysctl interface that can be used to prevent
> +  applications from enabling or disabling the relaxed ABI. The sysctl
> +  supports the following configuration options:
> +
> +  - **0**: disable the prctl(PR_SET_TAGGED_ADDR_CTRL) option to
> +    enable/disable the AArch64 Tagged Address ABI globally

This is clunky because it sounds like we're enabling the ABI for everybody,
where in actual fact we're enabling the controls for the ABI instead. It
also applies equally to PR_GET_TAGGED_ADDR_CTRL (but see below). Given that
we've already defined the prctl() above, I think we can just say:

  **0**: AArch64 Tagged Address ABI prctl() calls will return -EINVAL
  **1**: AArch64 Tagged Address ABI prctl() calls will behave as documented above.

> +  - **1** (Default): enable the prctl(PR_SET_TAGGED_ADDR_CTRL) option to
> +    enable/disable the AArch64 Tagged Address ABI globally
> +
> +  Note that this sysctl does not affect the status of the AArch64 Tagged
> +  Address ABI of the running processes.

Hmm, but it does mean that you can no longer ask if a previously running
process is using tags. Is that intentional?

> +When a process has successfully enabled the new ABI by invoking
> +prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE), the following
> +behaviours are guaranteed:

nit: this also applies to processes that have inherited the new ABI
bevaiour via fork() and haven't invoked the prctl() themselves.

> +- Every currently available syscall, except the cases mentioned in section

"currently available" is meaningless and should be removed

> +  3, can accept any valid tagged pointer. The same rule is applicable to
> +  any syscall introduced in the future.

Delete this last sentence.

> +- The syscall behaviour is undefined for non valid tagged pointers.

non valid => invalid

although this needs to be better defined, I think.

> +
> +- Every valid tagged pointer is expected to work as an untagged one.

What does that mean? Expected by who? What does "work" mean?

> +A definition of the meaning of tagged pointers on AArch64 can be found in:
> +Documentation/arm64/tagged-pointers.txt.

.txt => .rst

> +
> +3. AArch64 Tagged Address ABI Exceptions
> +-----------------------------------------
> +
> +The behaviour described in section 2, with particular reference to the
> +acceptance by the syscalls of any valid tagged pointer, is not applicable
> +to the following cases:

Jeez louise...

How about: "The following system call parameters must be untagged, regardless
of the ABI relaxation:"

> +
> +- mmap() addr parameter.
> +
> +- mremap() new_address parameter.
> +
> +- prctl(PR_SET_MM, ``*``, ...) other than arg2 PR_SET_MM_MAP and
> +  PR_SET_MM_MAP_SIZE.
> +
> +- prctl(PR_SET_MM, PR_SET_MM_MAP{,_SIZE}, ...) struct prctl_mm_map fields.

How did you generate this list and who will keep it up to date? How do you
know you haven't missed anything?

> +Any attempt to use non-zero tagged pointers will lead to undefined
> +behaviour.

In the tagged pointer document we're slightly more specific and say that
using non-zero address tags "may result in an error code being returned, a
(fatal) signal being rasied, or other modes of failure". Maybe reuse that?

> +4. Example of correct usage
> +---------------------------
> +.. code-block:: c
> +
> +   void main(void)
> +   {
> +           static int tbi_enabled = 0;
> +           unsigned long tag = 0;
> +

Some comments won't go amiss here.

> +           char *ptr = mmap(NULL, PAGE_SIZE, PROT_READ | PROT_WRITE,
> +                            MAP_ANONYMOUS, -1, 0);
> +
> +           if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE,
> +                     0, 0, 0) == 0)
> +                   tbi_enabled = 1;
> +
> +           if (ptr == (void *)-1) /* MAP_FAILED */
> +                   return -1;
> +
> +           if (tbi_enabled)
> +                   tag = rand() & 0xff;
> +
> +           ptr = (char *)((unsigned long)ptr | (tag << TAG_SHIFT));
> +
> +           *ptr = 'a';
> +
> +           ...
> +   }

Hmm, doesn't this snippet work today? You're not actually passing the
tagged pointer back to the kernel...

Will

Catalin Marinas Aug. 8, 2019, 5:27 p.m. UTC | #7

On Wed, Aug 07, 2019 at 01:38:16PM -0700, Dave Hansen wrote:
> On 8/7/19 8:53 AM, Catalin Marinas wrote:
> > +- mmap() done by the process itself (or its parent), where either:
> > +
> > +  - flags have the **MAP_ANONYMOUS** bit set
> > +  - the file descriptor refers to a regular file (including those returned
> > +    by memfd_create()) or **/dev/zero**
> 
> What's a "regular file"? ;)

We could make it more explicit like '(stat.st_mode & S_IFMT) == S_IFREG'
but it gets too verbose ;).

> > +- brk() system call done by the process itself (i.e. the heap area between
> > +  the initial location of the program break at process creation and its
> > +  current location).
> > +
> > +- any memory mapped by the kernel in the address space of the process
> > +  during creation and with the same restrictions as for mmap() above (e.g.
> > +  data, bss, stack).
> > +
> > +The AArch64 Tagged Address ABI is an opt-in feature and an application can
> > +control it via **prctl()** as follows:
> > +
> > +- **PR_SET_TAGGED_ADDR_CTRL**: enable or disable the AArch64 Tagged Address
> > +  ABI for the calling process.
> > +
> > +  The (unsigned int) arg2 argument is a bit mask describing the control mode
> > +  used:
> > +
> > +  - **PR_TAGGED_ADDR_ENABLE**: enable AArch64 Tagged Address ABI. Default
> > +    status is disabled.
> > +
> > +  The arguments arg3, arg4, and arg5 are ignored.
> 
> For previous prctl()'s, we've found that it's best to require that the
> unused arguments be 0.  Without that, apps are free to put garbage
> there, which makes extending the prctl to use other arguments impossible
> in the future.

We've had a bit of bikeshedding already:

http://lkml.kernel.org/r/[e-mail]

Extending the interface is still possible even with the current
proposal, by changing arg2 etc. We also don't seem to be consistent in
sys_prctl().

> Also, shouldn't this be converted over to an arch_prctl()?

What do you mean by arch_prctl()? We don't have such thing, apart from
maybe arch_prctl_spec_ctrl_*(). We achieve the same thing with the
{SET,GET}_TAGGED_ADDR_CTRL macros. They could be renamed to
arch_prctl_tagged_addr_{set,get} or something but I don't see much
point.

What would be better (for a separate patch series) is to clean up
sys_prctl() and move the arch-specific options into separate
arch_prctl() under arch/*/kernel/. But it's not really for this series.

> > +The prctl(PR_SET_TAGGED_ADDR_CTRL, ...) will return -EINVAL if the
> > +AArch64 Tagged Address ABI is not available
> > +(CONFIG_ARM64_TAGGED_ADDR_ABI disabled or sysctl abi.tagged_addr=0).
> > +
> > +The ABI properties set by the mechanism described above are inherited by
> > +threads of the same application and fork()'ed children but cleared by
> > +execve().
> 
> What is the scope of these prctl()'s?  Are they thread-scoped or
> process-scoped?  Can two threads in the same process run with different
> tagging ABI modes?

Good point. They are thread-scoped and this should be made clear in the
doc. Two threads can have different modes.

The expectation is that this is invoked early during process start (by
the dynamic loader or libc init) while in single-thread mode and
subsequent threads will inherit the same mode. However, other uses are
possible.

> > +Opting in (the prctl() option described above only) to or out of the
> > +AArch64 Tagged Address ABI can be disabled globally at runtime using the
> > +sysctl interface:
> > +
> > +- **abi.tagged_addr**: a new sysctl interface that can be used to prevent
> > +  applications from enabling or disabling the relaxed ABI. The sysctl
> > +  supports the following configuration options:
> > +
> > +  - **0**: disable the prctl(PR_SET_TAGGED_ADDR_CTRL) option to
> > +    enable/disable the AArch64 Tagged Address ABI globally
> > +
> > +  - **1** (Default): enable the prctl(PR_SET_TAGGED_ADDR_CTRL) option to
> > +    enable/disable the AArch64 Tagged Address ABI globally
> > +
> > +  Note that this sysctl does not affect the status of the AArch64 Tagged
> > +  Address ABI of the running processes.
> 
> Shouldn't the name be "abi.tagged_addr_control" or something?  It
> actually has *zero* direct effect on tagged addresses in the ABI.

Yeah, we could add a _ctrl suffix. I usually lack inspiration when
naming things.

> What's the reason for allowing it to be toggled at runtime like this?
> Wouldn't it make more sense to just have it be a boot option so you
> *know* what the state of individual processes is?

This was initially suggested by Vincenzo but I wasn't keen on having a
kernel command line option that affects the user ABI. Since then we went
through several incarnations and ended up with a default off for the
relaxed ABI with an opt-in prctl(). The reason behind default off is
that I'm not 100% confident the kernel won't break the relaxed ABI in
the future and I wouldn't want applications that don't use
top-byte-ignore (or tagged addresses) to inadvertently start using it.
The additional sysctl is to allow system administrators to block the
opt-in altogether. It also comes in handy for testing userspace
behaviour without rebooting.

That said, do we have a precedent for changing user ABI from the kernel
cmd line? 'noexec32', 'vsyscall' I think come close. With the prctl()
for opt-in, controlling this from the cmd line is not too bad (though my
preference is still for the sysctl).

> > +When a process has successfully enabled the new ABI by invoking
> > +prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE), the following
> > +behaviours are guaranteed:
> > +
> > +- Every currently available syscall, except the cases mentioned in section
> > +  3, can accept any valid tagged pointer. The same rule is applicable to
> > +  any syscall introduced in the future.
> > +
> > +- The syscall behaviour is undefined for non valid tagged pointers.
> 
> Do you really mean "undefined"?  I mean, a bad pointer is a bad pointer.
>  Why should it matter if it's a tagged bad pointer or an untagged bad
> pointer?

Szabolcs already replied here. We may have tagged pointers that can be
dereferenced just fine but being passed to the kernel may not be well
defined (e.g. some driver doing a find_vma() that fails unless it
explicitly untags the address). It's as undefined as the current
behaviour (without these patches) guarantees.

> ...
> > +A definition of the meaning of tagged pointers on AArch64 can be found in:
> > +Documentation/arm64/tagged-pointers.txt.
> > +
> > +3. AArch64 Tagged Address ABI Exceptions
> > +-----------------------------------------
> > +
> > +The behaviour described in section 2, with particular reference to the
> > +acceptance by the syscalls of any valid tagged pointer, is not applicable
> > +to the following cases:
> 
> This is saying things in a pretty roundabout manner.  Can't it just say:
>  "The following cases do not accept tagged pointers:"

I agree.

> > +- mmap() addr parameter.
> > +
> > +- mremap() new_address parameter.
> 
> Is munmap() missing?  Or was there a reason for leaving it out?

Szabolcs replied already here.

For a bit of history, I initially didn't want any of the address space
handling functions to accept tagged pointers but it got harder to
specify what this means that can be safely applied to future syscall
extensions. We then changed the approach to allow it everywhere with
some exclusions like mmap/mremap.

> > +- prctl(PR_SET_MM, ``*``, ...) other than arg2 PR_SET_MM_MAP and
> > +  PR_SET_MM_MAP_SIZE.
> > +
> > +- prctl(PR_SET_MM, PR_SET_MM_MAP{,_SIZE}, ...) struct prctl_mm_map fields.
> > +
> > +Any attempt to use non-zero tagged pointers will lead to undefined
> > +behaviour.
> 
> I wonder if you want to generalize this a bit.  I think you're saying
> that parts of the ABI that modify the *layout* of the address space
> never accept tagged pointers.

I guess our difficulty in specifying this may have been caused by
over-generalising. For example, madvise/mprotect came under the same
category but there is a use-case for malloc'ed pointers (and tagged) to
the kernel (e.g. MADV_DONTNEED). If we can restrict the meaning to
address space *layout* manipulation, we'd have mmap/mremap/munmap,
brk/sbrk, prctl(PR_SET_MM). Did I miss anything?. Other related syscalls
like mprotect/madvise preserve the layout while only changing permissions,
backing store, so the would be allowed to accept tags.

Open to feedback from others, especially libc/userspace folk. Ideally,
what I'd like is that when a new syscall is added (or extension to an
existing syscall), it should be fairly obvious to the user whether it
can take a tagged address or not (or maybe that's just not possible).

> > +4. Example of correct usage
> > +---------------------------
> > +.. code-block:: c
> > +
> > +   void main(void)
> > +   {
> > +           static int tbi_enabled = 0;
> > +           unsigned long tag = 0;
> > +
> > +           char *ptr = mmap(NULL, PAGE_SIZE, PROT_READ | PROT_WRITE,
> > +                            MAP_ANONYMOUS, -1, 0);
> > +
> > +           if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE,
> > +                     0, 0, 0) == 0)
> > +                   tbi_enabled = 1;
> > +
> > +           if (ptr == (void *)-1) /* MAP_FAILED */
> > +                   return -1;
> > +
> > +           if (tbi_enabled)
> > +                   tag = rand() & 0xff;
> > +
> > +           ptr = (char *)((unsigned long)ptr | (tag << TAG_SHIFT));
> > +
> > +           *ptr = 'a';
> > +
> > +           ...
> > +   }
> 
> It looks like the TAG_SHIFT and tag size are pretty baked into the
> aarch64 architecture.  But, are you confident that no future
> implementations will want different positions or sizes?  (obviously
> controlled by other TCR_EL1 bits)

For the top-byte-ignore (TBI), that's been baked in the architecture
since ARMv8.0 and we'll have to keep the backwards compatible mode. As
the name implies, it's the top byte of the address and that's what the
document above refers to.

With MTE, I can't exclude other configurations in the future but I'd
expect the kernel to present the option as a new HWCAP and the user to
explicitly opt in via a new prctl() flag. I seriously doubt we'd break
existing binaries. So, yes TAG_SHIFT may be different but so would the
prctl() above.

Thanks.

Dave Hansen Aug. 9, 2019, 2:10 p.m. UTC | #8

On 8/8/19 10:27 AM, Catalin Marinas wrote:
> On Wed, Aug 07, 2019 at 01:38:16PM -0700, Dave Hansen wrote:
> Extending the interface is still possible even with the current
> proposal, by changing arg2 etc. We also don't seem to be consistent in
> sys_prctl().

We are not consistent because it took a long time to learn this lesson,
but I think this is a best-practice that we follow for new ones.

>> Also, shouldn't this be converted over to an arch_prctl()?
> 
> What do you mean by arch_prctl()? We don't have such thing, apart from
> maybe arch_prctl_spec_ctrl_*(). We achieve the same thing with the
> {SET,GET}_TAGGED_ADDR_CTRL macros. They could be renamed to
> arch_prctl_tagged_addr_{set,get} or something but I don't see much
> point.

Silly me.  We have an x86-specific:

	SYSCALL_DEFINE2(arch_prctl, int , option, unsigned long , arg2)

I guess there's no ARM equivalent so you're stuck with the generic one.

> What would be better (for a separate patch series) is to clean up
> sys_prctl() and move the arch-specific options into separate
> arch_prctl() under arch/*/kernel/. But it's not really for this series.

I think it does make sense for truly arch-specific features to stay out
of the arch-generic prctl().  Yes, I know I've personally violated this
in the past. :)

>> What is the scope of these prctl()'s?  Are they thread-scoped or
>> process-scoped?  Can two threads in the same process run with different
>> tagging ABI modes?
> 
> Good point. They are thread-scoped and this should be made clear in the
> doc. Two threads can have different modes.
> 
> The expectation is that this is invoked early during process start (by
> the dynamic loader or libc init) while in single-thread mode and
> subsequent threads will inherit the same mode. However, other uses are
> possible.

If that's the expectation, it would be really nice to codify it.
Basically, you can't enable the feature if another thread is already
been forked off.

> That said, do we have a precedent for changing user ABI from the kernel
> cmd line? 'noexec32', 'vsyscall' I think come close. With the prctl()
> for opt-in, controlling this from the cmd line is not too bad (though my
> preference is still for the sysctl).

There are certainly user-visible things like being able to select
various CPU features.

>>> +When a process has successfully enabled the new ABI by invoking
>>> +prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE), the following
>>> +behaviours are guaranteed:
>>> +
>>> +- Every currently available syscall, except the cases mentioned in section
>>> +  3, can accept any valid tagged pointer. The same rule is applicable to
>>> +  any syscall introduced in the future.
>>> +
>>> +- The syscall behaviour is undefined for non valid tagged pointers.
>>
>> Do you really mean "undefined"?  I mean, a bad pointer is a bad pointer.
>>  Why should it matter if it's a tagged bad pointer or an untagged bad
>> pointer?
> 
> Szabolcs already replied here. We may have tagged pointers that can be
> dereferenced just fine but being passed to the kernel may not be well
> defined (e.g. some driver doing a find_vma() that fails unless it
> explicitly untags the address). It's as undefined as the current
> behaviour (without these patches) guarantees.

It might just be nicer to point out what this features *changes* about
invalid pointer handling, which is nothing. :)  Maybe:

	The syscall behaviour for invalid pointers is the same for both
	tagged and untagged pointers.

>>> +- prctl(PR_SET_MM, ``*``, ...) other than arg2 PR_SET_MM_MAP and
>>> +  PR_SET_MM_MAP_SIZE.
>>> +
>>> +- prctl(PR_SET_MM, PR_SET_MM_MAP{,_SIZE}, ...) struct prctl_mm_map fields.
>>> +
>>> +Any attempt to use non-zero tagged pointers will lead to undefined
>>> +behaviour.
>>
>> I wonder if you want to generalize this a bit.  I think you're saying
>> that parts of the ABI that modify the *layout* of the address space
>> never accept tagged pointers.
> 
> I guess our difficulty in specifying this may have been caused by
> over-generalising. For example, madvise/mprotect came under the same
> category but there is a use-case for malloc'ed pointers (and tagged) to
> the kernel (e.g. MADV_DONTNEED). If we can restrict the meaning to
> address space *layout* manipulation, we'd have mmap/mremap/munmap,
> brk/sbrk, prctl(PR_SET_MM). Did I miss anything?. Other related syscalls
> like mprotect/madvise preserve the layout while only changing permissions,
> backing store, so the would be allowed to accept tags.

shmat() comes to mind.  I also did a quick grep for mmap_sem taken for
write and didn't see anything else obvious jump out at me.

>> It looks like the TAG_SHIFT and tag size are pretty baked into the
>> aarch64 architecture.  But, are you confident that no future
>> implementations will want different positions or sizes?  (obviously
>> controlled by other TCR_EL1 bits)
> 
> For the top-byte-ignore (TBI), that's been baked in the architecture
> since ARMv8.0 and we'll have to keep the backwards compatible mode. As
> the name implies, it's the top byte of the address and that's what the
> document above refers to.
> 
> With MTE, I can't exclude other configurations in the future but I'd
> expect the kernel to present the option as a new HWCAP and the user to
> explicitly opt in via a new prctl() flag. I seriously doubt we'd break
> existing binaries. So, yes TAG_SHIFT may be different but so would the
> prctl() above.

Basically, what you have is a "turn tagging on" and "turn tagging off"
call which are binary: all on or all off.  How about exposing a mask:

	/* Replace hard-coded mask size/position: */
	unsigned long mask = prctl(GET_POSSIBLE_TAGGED_ADDR_BITS);

	if (mask == 0)
		// no tagging is supported obviously

	prctl(SET_TAGGED_ADDR_BITS, mask);

	// now userspace knows via 'mask' where the tag bits are

Andrew Murray Aug. 12, 2019, 10:46 a.m. UTC | #9

On Thu, Aug 08, 2019 at 06:04:24PM +0100, Will Deacon wrote:
> On Wed, Aug 07, 2019 at 04:53:20PM +0100, Catalin Marinas wrote:
> > From: Vincenzo Frascino <[e-mail]>
> > 
> > On arm64 the TCR_EL1.TBI0 bit has been always enabled hence
> > the userspace (EL0) is allowed to set a non-zero value in the
> > top byte but the resulting pointers are not allowed at the
> > user-kernel syscall ABI boundary.
> > 
> > With the relaxed ABI proposed through this document, it is now possible
> > to pass tagged pointers to the syscalls, when these pointers are in
> > memory ranges obtained by an anonymous (MAP_ANONYMOUS) mmap().
> > 
> > This change in the ABI requires a mechanism to requires the userspace
> > to opt-in to such an option.
> > 
> > Specify and document the way in which sysctl and prctl() can be used
> > in combination to allow the userspace to opt-in this feature.
> > 
> > Cc: Will Deacon <[e-mail]>
> > Cc: Andrey Konovalov <[e-mail]>
> > Cc: Szabolcs Nagy <[e-mail]>
> > Cc: Kevin Brodsky <[e-mail]>
> > Signed-off-by: Vincenzo Frascino <[e-mail]>
> > [[e-mail]: some rewording, dropped MAP_PRIVATE]
> > Signed-off-by: Catalin Marinas <[e-mail]>
> > ---
> >  Documentation/arm64/tagged-address-abi.rst | 151 +++++++++++++++++++++
> >  1 file changed, 151 insertions(+)
> >  create mode 100644 Documentation/arm64/tagged-address-abi.rst
> > 
> > diff --git a/Documentation/arm64/tagged-address-abi.rst b/Documentation/arm64/tagged-address-abi.rst
> > new file mode 100644
> > index 000000000000..f91a5d2ac865
> > --- /dev/null
> > +++ b/Documentation/arm64/tagged-address-abi.rst
> > @@ -0,0 +1,151 @@
> > +==========================
> > +AArch64 TAGGED ADDRESS ABI
> > +==========================
> > +
> > +Author: Vincenzo Frascino <[e-mail]>
> > +
> > +Date: 25 July 2019
> > +
> > +This document describes the usage and semantics of the Tagged Address
> > +ABI on AArch64 Linux.
> > +
> > +1. Introduction
> > +---------------
> > +
> > +On AArch64 the TCR_EL1.TBI0 bit has always been enabled, allowing userspace
> > +(EL0) to perform memory accesses through 64-bit pointers with a non-zero
> > +top byte. Such tagged pointers, however, were not allowed at the
> > +user-kernel syscall ABI boundary.
> 
> I think we should drop the temporal language, so:
> 
>   "has always been enabled" => "is set by the kernel"
>   "were not allowed" => "are not allowed by default"
> 
> > +
> > +This document describes the relaxation of the syscall ABI that allows
> > +userspace to pass certain tagged pointers to kernel syscalls, as described
> > +in section 2.
> > +
> > +2. AArch64 Tagged Address ABI
> > +-----------------------------
> > +
> > +From the kernel syscall interface perspective and for the purposes of this
> > +document, a "valid tagged pointer" is a pointer with a potentially non-zero
> > +top-byte that references an address in the user process address space
> > +obtained in one of the following ways:
> > +
> > +- mmap() done by the process itself (or its parent), where either:
> > +
> > +  - flags have the **MAP_ANONYMOUS** bit set
> > +  - the file descriptor refers to a regular file (including those returned
> > +    by memfd_create()) or **/dev/zero**
> > +
> > +- brk() system call done by the process itself (i.e. the heap area between
> > +  the initial location of the program break at process creation and its
> > +  current location).
> > +
> > +- any memory mapped by the kernel in the address space of the process
> > +  during creation and with the same restrictions as for mmap() above (e.g.
> > +  data, bss, stack).
> > +
> > +The AArch64 Tagged Address ABI is an opt-in feature and an application can
> > +control it via **prctl()** as follows:
> > +
> > +- **PR_SET_TAGGED_ADDR_CTRL**: enable or disable the AArch64 Tagged Address
> > +  ABI for the calling process.
> > +
> > +  The (unsigned int) arg2 argument is a bit mask describing the control mode
> > +  used:
> > +
> > +  - **PR_TAGGED_ADDR_ENABLE**: enable AArch64 Tagged Address ABI. Default
> > +    status is disabled.
> > +
> > +  The arguments arg3, arg4, and arg5 are ignored.
> > +
> > +- **PR_GET_TAGGED_ADDR_CTRL**: get the status of the AArch64 Tagged Address
> > +  ABI for the calling process.
> > +
> > +  The arguments arg2, arg3, arg4, and arg5 are ignored.
> 
> I agree with Dave (H) that we should require these to be zero. We may be
> able to use arg2 to namespace things for PR_SET_TAGGED_ADDR_CTRL, but for
> PR_GET_TAGGED_ADDR_CTRL we'd have to add a new prctl if we wanted to extend
> it otherwise.
> 
> > +The prctl(PR_SET_TAGGED_ADDR_CTRL, ...) will return -EINVAL if the
> 
> *The* prctl? Maybe "Calling prctl(..." is better?
> 
> > +AArch64 Tagged Address ABI is not available
> > +(CONFIG_ARM64_TAGGED_ADDR_ABI disabled or sysctl abi.tagged_addr=0).
> 
> drop the brackets and say "because CONFIG_... is disabled or ..".
> 
> > +
> > +The ABI properties set by the mechanism described above are inherited by
> > +threads of the same application and fork()'ed children but cleared by
> > +execve().
> 
> Maybe just exec() here, since there are other flavours we shouldn't need to
> enumerate.
> 
> > +Opting in (the prctl() option described above only) to or out of the
> > +AArch64 Tagged Address ABI can be disabled globally at runtime using the
> > +sysctl interface:
> 
> This sentence reads really badly thanks to the random bracketed part.
> 
> > +
> > +- **abi.tagged_addr**: a new sysctl interface that can be used to prevent
> > +  applications from enabling or disabling the relaxed ABI. The sysctl
> > +  supports the following configuration options:
> > +
> > +  - **0**: disable the prctl(PR_SET_TAGGED_ADDR_CTRL) option to
> > +    enable/disable the AArch64 Tagged Address ABI globally
> 
> This is clunky because it sounds like we're enabling the ABI for everybody,
> where in actual fact we're enabling the controls for the ABI instead. It
> also applies equally to PR_GET_TAGGED_ADDR_CTRL (but see below). Given that
> we've already defined the prctl() above, I think we can just say:
> 
>   **0**: AArch64 Tagged Address ABI prctl() calls will return -EINVAL
>   **1**: AArch64 Tagged Address ABI prctl() calls will behave as documented above.
> 
> > +  - **1** (Default): enable the prctl(PR_SET_TAGGED_ADDR_CTRL) option to
> > +    enable/disable the AArch64 Tagged Address ABI globally
> > +
> > +  Note that this sysctl does not affect the status of the AArch64 Tagged
> > +  Address ABI of the running processes.
> 
> Hmm, but it does mean that you can no longer ask if a previously running
> process is using tags. Is that intentional?
> 
> > +When a process has successfully enabled the new ABI by invoking
> > +prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE), the following
> > +behaviours are guaranteed:
> 
> nit: this also applies to processes that have inherited the new ABI
> bevaiour via fork() and haven't invoked the prctl() themselves.
> 
> > +- Every currently available syscall, except the cases mentioned in section
> 
> "currently available" is meaningless and should be removed
> 
> > +  3, can accept any valid tagged pointer. The same rule is applicable to
> > +  any syscall introduced in the future.
> 
> Delete this last sentence.
> 
> > +- The syscall behaviour is undefined for non valid tagged pointers.
> 
> non valid => invalid
> 
> although this needs to be better defined, I think.
> 
> > +
> > +- Every valid tagged pointer is expected to work as an untagged one.
> 
> What does that mean? Expected by who? What does "work" mean?
> 
> > +A definition of the meaning of tagged pointers on AArch64 can be found in:
> > +Documentation/arm64/tagged-pointers.txt.
> 
> .txt => .rst
> 
> > +
> > +3. AArch64 Tagged Address ABI Exceptions
> > +-----------------------------------------
> > +
> > +The behaviour described in section 2, with particular reference to the
> > +acceptance by the syscalls of any valid tagged pointer, is not applicable
> > +to the following cases:
> 
> Jeez louise...
> 
> How about: "The following system call parameters must be untagged, regardless
> of the ABI relaxation:"
> 
> > +
> > +- mmap() addr parameter.
> > +
> > +- mremap() new_address parameter.
> > +
> > +- prctl(PR_SET_MM, ``*``, ...) other than arg2 PR_SET_MM_MAP and
> > +  PR_SET_MM_MAP_SIZE.
> > +
> > +- prctl(PR_SET_MM, PR_SET_MM_MAP{,_SIZE}, ...) struct prctl_mm_map fields.
> 
> How did you generate this list and who will keep it up to date? How do you
> know you haven't missed anything?

What about shared memory system calls: shmat, shmdt? The latest "arm64: untag
user pointers passed to the kernel" series doesn't untag these, thus we should
indicate here that these too are no supported.

Thanks,

Andrew Murray

> 
> > +Any attempt to use non-zero tagged pointers will lead to undefined
> > +behaviour.
> 
> In the tagged pointer document we're slightly more specific and say that
> using non-zero address tags "may result in an error code being returned, a
> (fatal) signal being rasied, or other modes of failure". Maybe reuse that?
> 
> > +4. Example of correct usage
> > +---------------------------
> > +.. code-block:: c
> > +
> > +   void main(void)
> > +   {
> > +           static int tbi_enabled = 0;
> > +           unsigned long tag = 0;
> > +
> 
> Some comments won't go amiss here.
> 
> > +           char *ptr = mmap(NULL, PAGE_SIZE, PROT_READ | PROT_WRITE,
> > +                            MAP_ANONYMOUS, -1, 0);
> > +
> > +           if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE,
> > +                     0, 0, 0) == 0)
> > +                   tbi_enabled = 1;
> > +
> > +           if (ptr == (void *)-1) /* MAP_FAILED */
> > +                   return -1;
> > +
> > +           if (tbi_enabled)
> > +                   tag = rand() & 0xff;
> > +
> > +           ptr = (char *)((unsigned long)ptr | (tag << TAG_SHIFT));
> > +
> > +           *ptr = 'a';
> > +
> > +           ...
> > +   }
> 
> Hmm, doesn't this snippet work today? You're not actually passing the
> tagged pointer back to the kernel...
> 
> Will
> 
> _______________________________________________
> linux-arm-kernel mailing list
> [e-mail]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Catalin Marinas Aug. 12, 2019, 5:17 p.m. UTC | #10

On Mon, Aug 12, 2019 at 11:46:06AM +0100, Andrew Murray wrote:
> On Thu, Aug 08, 2019 at 06:04:24PM +0100, Will Deacon wrote:
> > On Wed, Aug 07, 2019 at 04:53:20PM +0100, Catalin Marinas wrote:
> > > +
> > > +- mmap() addr parameter.
> > > +
> > > +- mremap() new_address parameter.
> > > +
> > > +- prctl(PR_SET_MM, ``*``, ...) other than arg2 PR_SET_MM_MAP and
> > > +  PR_SET_MM_MAP_SIZE.
> > > +
> > > +- prctl(PR_SET_MM, PR_SET_MM_MAP{,_SIZE}, ...) struct prctl_mm_map fields.
> > 
> > How did you generate this list and who will keep it up to date? How do you
> > know you haven't missed anything?
> 
> What about shared memory system calls: shmat, shmdt? The latest "arm64: untag
> user pointers passed to the kernel" series doesn't untag these, thus we should
> indicate here that these too are no supported.

Yes. We dropped them from a previous version of the series but they've
never been documented. I'll add something (unless someone has a real
use-case for using tags with shmat/shmdt).

Catalin Marinas Aug. 12, 2019, 5:36 p.m. UTC | #11

On Fri, Aug 09, 2019 at 07:10:18AM -0700, Dave Hansen wrote:
> On 8/8/19 10:27 AM, Catalin Marinas wrote:
> > On Wed, Aug 07, 2019 at 01:38:16PM -0700, Dave Hansen wrote:
> >> Also, shouldn't this be converted over to an arch_prctl()?
> > 
> > What do you mean by arch_prctl()? We don't have such thing, apart from
> > maybe arch_prctl_spec_ctrl_*(). We achieve the same thing with the
> > {SET,GET}_TAGGED_ADDR_CTRL macros. They could be renamed to
> > arch_prctl_tagged_addr_{set,get} or something but I don't see much
> > point.
> 
> Silly me.  We have an x86-specific:
> 
> 	SYSCALL_DEFINE2(arch_prctl, int , option, unsigned long , arg2)
> 
> I guess there's no ARM equivalent so you're stuck with the generic one.
> 
> > What would be better (for a separate patch series) is to clean up
> > sys_prctl() and move the arch-specific options into separate
> > arch_prctl() under arch/*/kernel/. But it's not really for this series.
> 
> I think it does make sense for truly arch-specific features to stay out
> of the arch-generic prctl().  Yes, I know I've personally violated this
> in the past. :)

Maybe Dave M could revive his prctl() clean-up patches which moves the
arch specific cases to the corresponding arch/*/ code

> >> What is the scope of these prctl()'s?  Are they thread-scoped or
> >> process-scoped?  Can two threads in the same process run with different
> >> tagging ABI modes?
> > 
> > Good point. They are thread-scoped and this should be made clear in the
> > doc. Two threads can have different modes.
> > 
> > The expectation is that this is invoked early during process start (by
> > the dynamic loader or libc init) while in single-thread mode and
> > subsequent threads will inherit the same mode. However, other uses are
> > possible.
> 
> If that's the expectation, it would be really nice to codify it.
> Basically, you can't enable the feature if another thread is already
> been forked off.

Well, that's my expectation but I'm not a userspace developer. I don't
think there is any good reason to prevent it. It doesn't cost us
anything to support in the kernel, other than making the documentation
clearer.

> >>> +When a process has successfully enabled the new ABI by invoking
> >>> +prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE), the following
> >>> +behaviours are guaranteed:
> >>> +
> >>> +- Every currently available syscall, except the cases mentioned in section
> >>> +  3, can accept any valid tagged pointer. The same rule is applicable to
> >>> +  any syscall introduced in the future.
> >>> +
> >>> +- The syscall behaviour is undefined for non valid tagged pointers.
> >>
> >> Do you really mean "undefined"?  I mean, a bad pointer is a bad pointer.
> >>  Why should it matter if it's a tagged bad pointer or an untagged bad
> >> pointer?
> > 
> > Szabolcs already replied here. We may have tagged pointers that can be
> > dereferenced just fine but being passed to the kernel may not be well
> > defined (e.g. some driver doing a find_vma() that fails unless it
> > explicitly untags the address). It's as undefined as the current
> > behaviour (without these patches) guarantees.
> 
> It might just be nicer to point out what this features *changes* about
> invalid pointer handling, which is nothing. :)  Maybe:
> 
> 	The syscall behaviour for invalid pointers is the same for both
> 	tagged and untagged pointers.

Good point.

> >>> +- prctl(PR_SET_MM, ``*``, ...) other than arg2 PR_SET_MM_MAP and
> >>> +  PR_SET_MM_MAP_SIZE.
> >>> +
> >>> +- prctl(PR_SET_MM, PR_SET_MM_MAP{,_SIZE}, ...) struct prctl_mm_map fields.
> >>> +
> >>> +Any attempt to use non-zero tagged pointers will lead to undefined
> >>> +behaviour.
> >>
> >> I wonder if you want to generalize this a bit.  I think you're saying
> >> that parts of the ABI that modify the *layout* of the address space
> >> never accept tagged pointers.
> > 
> > I guess our difficulty in specifying this may have been caused by
> > over-generalising. For example, madvise/mprotect came under the same
> > category but there is a use-case for malloc'ed pointers (and tagged) to
> > the kernel (e.g. MADV_DONTNEED). If we can restrict the meaning to
> > address space *layout* manipulation, we'd have mmap/mremap/munmap,
> > brk/sbrk, prctl(PR_SET_MM). Did I miss anything?. Other related syscalls
> > like mprotect/madvise preserve the layout while only changing permissions,
> > backing store, so the would be allowed to accept tags.
> 
> shmat() comes to mind.  I also did a quick grep for mmap_sem taken for
> write and didn't see anything else obvious jump out at me.

I'll document shmat() as not supported, together with the prctl().

As I submitted a fixup already, I propose that we allow tagged pointers
on mmap/munmap/mremap/brk. It makes the documentation simpler ;) (and
the user understanding of what is and is not allowed).

> >> It looks like the TAG_SHIFT and tag size are pretty baked into the
> >> aarch64 architecture.  But, are you confident that no future
> >> implementations will want different positions or sizes?  (obviously
> >> controlled by other TCR_EL1 bits)
> > 
> > For the top-byte-ignore (TBI), that's been baked in the architecture
> > since ARMv8.0 and we'll have to keep the backwards compatible mode. As
> > the name implies, it's the top byte of the address and that's what the
> > document above refers to.
> > 
> > With MTE, I can't exclude other configurations in the future but I'd
> > expect the kernel to present the option as a new HWCAP and the user to
> > explicitly opt in via a new prctl() flag. I seriously doubt we'd break
> > existing binaries. So, yes TAG_SHIFT may be different but so would the
> > prctl() above.
> 
> Basically, what you have is a "turn tagging on" and "turn tagging off"
> call which are binary: all on or all off.  How about exposing a mask:
> 
> 	/* Replace hard-coded mask size/position: */
> 	unsigned long mask = prctl(GET_POSSIBLE_TAGGED_ADDR_BITS);
> 
> 	if (mask == 0)
> 		// no tagging is supported obviously
> 
> 	prctl(SET_TAGGED_ADDR_BITS, mask);
> 
> 	// now userspace knows via 'mask' where the tag bits are

For the actual hardware memory tagging, maybe we could get the possible
bits but for TBI, as I said above, that's baked into the architecture. I
don't think it's worth the effort of getting a mask as I don't see ARM
changing this without breaking existing software. Even compiler support
like hwasan relies on the 8-bit TBI.

Dave Martin Aug. 13, 2019, 11:10 a.m. UTC | #12

On Mon, Aug 12, 2019 at 06:36:12PM +0100, Catalin Marinas wrote:
> On Fri, Aug 09, 2019 at 07:10:18AM -0700, Dave Hansen wrote:
> > On 8/8/19 10:27 AM, Catalin Marinas wrote:
> > > On Wed, Aug 07, 2019 at 01:38:16PM -0700, Dave Hansen wrote:
> > >> Also, shouldn't this be converted over to an arch_prctl()?
> > > 
> > > What do you mean by arch_prctl()? We don't have such thing, apart from
> > > maybe arch_prctl_spec_ctrl_*(). We achieve the same thing with the
> > > {SET,GET}_TAGGED_ADDR_CTRL macros. They could be renamed to
> > > arch_prctl_tagged_addr_{set,get} or something but I don't see much
> > > point.
> > 
> > Silly me.  We have an x86-specific:
> > 
> > 	SYSCALL_DEFINE2(arch_prctl, int , option, unsigned long , arg2)
> > 
> > I guess there's no ARM equivalent so you're stuck with the generic one.
> > 
> > > What would be better (for a separate patch series) is to clean up
> > > sys_prctl() and move the arch-specific options into separate
> > > arch_prctl() under arch/*/kernel/. But it's not really for this series.
> > 
> > I think it does make sense for truly arch-specific features to stay out
> > of the arch-generic prctl().  Yes, I know I've personally violated this
> > in the past. :)
> 
> Maybe Dave M could revive his prctl() clean-up patches which moves the
> arch specific cases to the corresponding arch/*/ code

I'll try to take a look at it.

> > >> What is the scope of these prctl()'s?  Are they thread-scoped or
> > >> process-scoped?  Can two threads in the same process run with different
> > >> tagging ABI modes?
> > > 
> > > Good point. They are thread-scoped and this should be made clear in the
> > > doc. Two threads can have different modes.
> > > 
> > > The expectation is that this is invoked early during process start (by
> > > the dynamic loader or libc init) while in single-thread mode and
> > > subsequent threads will inherit the same mode. However, other uses are
> > > possible.
> > 
> > If that's the expectation, it would be really nice to codify it.
> > Basically, you can't enable the feature if another thread is already
> > been forked off.
> 
> Well, that's my expectation but I'm not a userspace developer. I don't
> think there is any good reason to prevent it. It doesn't cost us
> anything to support in the kernel, other than making the documentation
> clearer.

This came up for SVE and eventually we didn't bother, partly because the
kernel doesn't fully know what userspace is trying to do, in general.

If userspace has some kind of worker threads that run specific code,
they could legitimately interface differently with the kernel compared
with, say, the main thread.  This model already exists for e.g.,
seccomp.

In any case, I think it's not up to the kernel to dictate how user
runtimes initialise themselves.

[...]

Cheers
---Dave

Patch

diff --git a/Documentation/arm64/tagged-address-abi.rst b/Documentation/arm64/tagged-address-abi.rst
new file mode 100644
index 000000000000..f91a5d2ac865
--- /dev/null
+++ b/Documentation/arm64/tagged-address-abi.rst
@@ -0,0 +1,151 @@ 
+==========================
+AArch64 TAGGED ADDRESS ABI
+==========================
+
+Author: Vincenzo Frascino <[e-mail]>
+
+Date: 25 July 2019
+
+This document describes the usage and semantics of the Tagged Address
+ABI on AArch64 Linux.
+
+1. Introduction
+---------------
+
+On AArch64 the TCR_EL1.TBI0 bit has always been enabled, allowing userspace
+(EL0) to perform memory accesses through 64-bit pointers with a non-zero
+top byte. Such tagged pointers, however, were not allowed at the
+user-kernel syscall ABI boundary.
+
+This document describes the relaxation of the syscall ABI that allows
+userspace to pass certain tagged pointers to kernel syscalls, as described
+in section 2.
+
+2. AArch64 Tagged Address ABI
+-----------------------------
+
+From the kernel syscall interface perspective and for the purposes of this
+document, a "valid tagged pointer" is a pointer with a potentially non-zero
+top-byte that references an address in the user process address space
+obtained in one of the following ways:
+
+- mmap() done by the process itself (or its parent), where either:
+
+  - flags have the **MAP_ANONYMOUS** bit set
+  - the file descriptor refers to a regular file (including those returned
+    by memfd_create()) or **/dev/zero**
+
+- brk() system call done by the process itself (i.e. the heap area between
+  the initial location of the program break at process creation and its
+  current location).
+
+- any memory mapped by the kernel in the address space of the process
+  during creation and with the same restrictions as for mmap() above (e.g.
+  data, bss, stack).
+
+The AArch64 Tagged Address ABI is an opt-in feature and an application can
+control it via **prctl()** as follows:
+
+- **PR_SET_TAGGED_ADDR_CTRL**: enable or disable the AArch64 Tagged Address
+  ABI for the calling process.
+
+  The (unsigned int) arg2 argument is a bit mask describing the control mode
+  used:
+
+  - **PR_TAGGED_ADDR_ENABLE**: enable AArch64 Tagged Address ABI. Default
+    status is disabled.
+
+  The arguments arg3, arg4, and arg5 are ignored.
+
+- **PR_GET_TAGGED_ADDR_CTRL**: get the status of the AArch64 Tagged Address
+  ABI for the calling process.
+
+  The arguments arg2, arg3, arg4, and arg5 are ignored.
+
+The prctl(PR_SET_TAGGED_ADDR_CTRL, ...) will return -EINVAL if the
+AArch64 Tagged Address ABI is not available
+(CONFIG_ARM64_TAGGED_ADDR_ABI disabled or sysctl abi.tagged_addr=0).
+
+The ABI properties set by the mechanism described above are inherited by
+threads of the same application and fork()'ed children but cleared by
+execve().
+
+Opting in (the prctl() option described above only) to or out of the
+AArch64 Tagged Address ABI can be disabled globally at runtime using the
+sysctl interface:
+
+- **abi.tagged_addr**: a new sysctl interface that can be used to prevent
+  applications from enabling or disabling the relaxed ABI. The sysctl
+  supports the following configuration options:
+
+  - **0**: disable the prctl(PR_SET_TAGGED_ADDR_CTRL) option to
+    enable/disable the AArch64 Tagged Address ABI globally
+
+  - **1** (Default): enable the prctl(PR_SET_TAGGED_ADDR_CTRL) option to
+    enable/disable the AArch64 Tagged Address ABI globally
+
+  Note that this sysctl does not affect the status of the AArch64 Tagged
+  Address ABI of the running processes.
+
+When a process has successfully enabled the new ABI by invoking
+prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE), the following
+behaviours are guaranteed:
+
+- Every currently available syscall, except the cases mentioned in section
+  3, can accept any valid tagged pointer. The same rule is applicable to
+  any syscall introduced in the future.
+
+- The syscall behaviour is undefined for non valid tagged pointers.
+
+- Every valid tagged pointer is expected to work as an untagged one.
+
+A definition of the meaning of tagged pointers on AArch64 can be found in:
+Documentation/arm64/tagged-pointers.txt.
+
+3. AArch64 Tagged Address ABI Exceptions
+-----------------------------------------
+
+The behaviour described in section 2, with particular reference to the
+acceptance by the syscalls of any valid tagged pointer, is not applicable
+to the following cases:
+
+- mmap() addr parameter.
+
+- mremap() new_address parameter.
+
+- prctl(PR_SET_MM, ``*``, ...) other than arg2 PR_SET_MM_MAP and
+  PR_SET_MM_MAP_SIZE.
+
+- prctl(PR_SET_MM, PR_SET_MM_MAP{,_SIZE}, ...) struct prctl_mm_map fields.
+
+Any attempt to use non-zero tagged pointers will lead to undefined
+behaviour.
+
+4. Example of correct usage
+---------------------------
+.. code-block:: c
+
+   void main(void)
+   {
+           static int tbi_enabled = 0;
+           unsigned long tag = 0;
+
+           char *ptr = mmap(NULL, PAGE_SIZE, PROT_READ | PROT_WRITE,
+                            MAP_ANONYMOUS, -1, 0);
+
+           if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE,
+                     0, 0, 0) == 0)
+                   tbi_enabled = 1;
+
+           if (ptr == (void *)-1) /* MAP_FAILED */
+                   return -1;
+
+           if (tbi_enabled)
+                   tag = rand() & 0xff;
+
+           ptr = (char *)((unsigned long)ptr | (tag << TAG_SHIFT));
+
+           *ptr = 'a';
+
+           ...
+   }