Linux Documentation
 help / color / mirror / Atom feed
* [PATCH v6 0/2] arm64 relaxed ABI
       [not found] <cover.1563904656.git.andreyknvl@google.com>
@ 2019-07-25 13:50 ` Vincenzo Frascino
  2019-07-25 13:50   ` [PATCH v6 1/2] arm64: Define Documentation/arm64/tagged-address-abi.rst Vincenzo Frascino
  2019-07-25 13:50   ` [PATCH v6 2/2] arm64: Relax Documentation/arm64/tagged-pointers.rst Vincenzo Frascino
  0 siblings, 2 replies; 10+ messages in thread
From: Vincenzo Frascino @ 2019-07-25 13:50 UTC (permalink / raw)
  To: linux-arm-kernel, linux-doc, linux-mm, linux-arch,
	linux-kselftest, linux-kernel
  Cc: vincenzo.frascino, Catalin Marinas, Will Deacon, Andrey Konovalov,
	Szabolcs Nagy

On arm64 the TCR_EL1.TBI0 bit has been always enabled on the arm64 kernel,
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.

This patchset proposes a relaxation of the ABI with which it is possible
to pass tagged tagged pointers to the syscalls, when these pointers are in
memory ranges obtained as described in tagged-address-abi.txt contained in
this patch series.

Since it is not desirable to relax the ABI to allow tagged user addresses
into the kernel indiscriminately, this patchset documents a new sysctl
interface (/proc/sys/abi/tagged_addr) that is used to prevent the applications
from enabling the relaxed ABI and a new prctl() interface that can be used to
enable or disable the relaxed ABI.

This patchset should be merged together with [1].

[1] https://patchwork.kernel.org/cover/10674351/

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
CC: Andrey Konovalov <andreyknvl@google.com>
Cc: Szabolcs Nagy <szabolcs.nagy@arm.com>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>

Vincenzo Frascino (2):
  arm64: Define Documentation/arm64/tagged-address-abi.rst
  arm64: Relax Documentation/arm64/tagged-pointers.rst

 Documentation/arm64/tagged-address-abi.rst | 148 +++++++++++++++++++++
 Documentation/arm64/tagged-pointers.rst    |  23 +++-
 2 files changed, 164 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/arm64/tagged-address-abi.rst

-- 
2.22.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v6 1/2] arm64: Define Documentation/arm64/tagged-address-abi.rst
  2019-07-25 13:50 ` [PATCH v6 0/2] arm64 relaxed ABI Vincenzo Frascino
@ 2019-07-25 13:50   ` Vincenzo Frascino
  2019-07-30 10:32     ` Kevin Brodsky
  2019-07-31 16:43     ` Dave Hansen
  2019-07-25 13:50   ` [PATCH v6 2/2] arm64: Relax Documentation/arm64/tagged-pointers.rst Vincenzo Frascino
  1 sibling, 2 replies; 10+ messages in thread
From: Vincenzo Frascino @ 2019-07-25 13:50 UTC (permalink / raw)
  To: linux-arm-kernel, linux-doc, linux-mm, linux-arch,
	linux-kselftest, linux-kernel
  Cc: vincenzo.frascino, Catalin Marinas, Will Deacon, Andrey Konovalov,
	Szabolcs Nagy

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: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
CC: Andrey Konovalov <andreyknvl@google.com>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
Acked-by: Szabolcs Nagy <szabolcs.nagy@arm.com>
---
 Documentation/arm64/tagged-address-abi.rst | 148 +++++++++++++++++++++
 1 file changed, 148 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..a8ecb991de82
--- /dev/null
+++ b/Documentation/arm64/tagged-address-abi.rst
@@ -0,0 +1,148 @@
+========================
+ARM64 TAGGED ADDRESS ABI
+========================
+
+Author: Vincenzo Frascino <vincenzo.frascino@arm.com>
+
+Date: 25 July 2019
+
+This document describes the usage and semantics of the Tagged Address
+ABI on arm64.
+
+1. Introduction
+---------------
+
+On arm64 the TCR_EL1.TBI0 bit has always been enabled on the kernel, hence
+the userspace (EL0) is entitled to perform a user memory access through a
+64-bit pointer with a non-zero top byte but the resulting pointers are not
+allowed at the user-kernel syscall ABI boundary.
+
+This document describes a relaxation of the ABI that makes it possible to
+to pass tagged pointers to the syscalls, when these pointers are in memory
+ranges obtained as described in section 2.
+
+Since it is not desirable to relax the ABI to allow tagged user addresses
+into the kernel indiscriminately, arm64 provides a new sysctl interface
+(/proc/sys/abi/tagged_addr) that is used to prevent the applications from
+enabling the relaxed ABI and a new prctl() interface that can be used to
+enable or disable the relaxed ABI.
+A detailed description of the newly introduced mechanisms will be provided
+in section 2.
+
+2. ARM64 Tagged Address ABI
+---------------------------
+
+From the kernel syscall interface perspective, we define, for the purposes
+of this document, a "valid tagged pointer" as a pointer that either has a
+zero value set in the top byte or has a non-zero value, is in memory ranges
+privately owned by a userspace process and is obtained in one of the
+following ways:
+- mmap() done by the process itself, where either:
+
+  - flags have **MAP_PRIVATE** and **MAP_ANONYMOUS**
+  - flags have **MAP_PRIVATE** and the file descriptor refers to a regular
+    file 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 process's address space during
+  creation and with the same restrictions as for mmap() (e.g. data, bss,
+  stack).
+
+The ARM64 Tagged Address ABI is an opt-in feature, and an application can
+control it using the following:
+
+- **/proc/sys/abi/tagged_addr**: a new sysctl interface that can be used to
+  prevent the applications from enabling the access to the relaxed ABI.
+  The sysctl supports the following configuration options:
+
+  - **0**: Disable the access to the ARM64 Tagged Address ABI for all
+    the applications.
+  - **1** (Default): Enable the access to the ARM64 Tagged Address ABI for
+    all the applications.
+
+   If the access to the ARM64 Tagged Address ABI is disabled at a certain
+   point in time, all the applications that were using tagging before this
+   event occurs, will continue to use tagging.
+- **prctl()s**:
+
+  - **PR_SET_TAGGED_ADDR_CTRL**: Invoked by a process, can be used to enable or
+    disable its access to the ARM64 Tagged Address ABI.
+
+    The (unsigned int) arg2 argument is a bit mask describing the control mode
+    used:
+
+    - **PR_TAGGED_ADDR_ENABLE**: Enable ARM64 Tagged Address ABI.
+
+    The prctl(PR_SET_TAGGED_ADDR_CTRL, ...) will return -EINVAL if the ARM64
+    Tagged Address ABI is not available.
+
+    The arguments arg3, arg4, and arg5 are ignored.
+  - **PR_GET_TAGGED_ADDR_CTRL**: can be used to check the status of the Tagged
+    Address ABI.
+
+    The arguments arg2, arg3, arg4, and arg5 are ignored.
+
+The ABI properties set by the mechanisms described above are inherited by threads
+of the same application and fork()'ed children but cleared by execve().
+
+When a process has successfully opted into the new ABI by invoking
+PR_SET_TAGGED_ADDR_CTRL prctl(), this guarantees the following behaviours:
+
+ - 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.
+ - If a non valid tagged pointer is passed to a syscall then the behaviour
+   is undefined.
+ - Every valid tagged pointer is expected to work as an untagged one.
+ - The kernel preserves any valid tagged pointer and returns it to the
+   userspace unchanged (i.e. on syscall return) in all the cases except the
+   ones documented in the "Preserving tags" section of tagged-pointers.txt.
+
+A definition of the meaning of tagged pointers on arm64 can be found in:
+Documentation/arm64/tagged-pointers.txt.
+
+3. ARM64 Tagged Address ABI Exceptions
+--------------------------------------
+
+The behaviours described in section 2, with particular reference to the
+acceptance by the syscalls of any valid tagged pointer are not applicable
+to the following cases:
+
+ - mmap() addr parameter.
+ - mremap() new_address parameter.
+ - prctl(PR_SET_MM, PR_SET_MM_MAP, ...) struct prctl_mm_map fields.
+ - 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';
+
+           ...
+   }
+
-- 
2.22.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v6 2/2] arm64: Relax Documentation/arm64/tagged-pointers.rst
  2019-07-25 13:50 ` [PATCH v6 0/2] arm64 relaxed ABI Vincenzo Frascino
  2019-07-25 13:50   ` [PATCH v6 1/2] arm64: Define Documentation/arm64/tagged-address-abi.rst Vincenzo Frascino
@ 2019-07-25 13:50   ` Vincenzo Frascino
  1 sibling, 0 replies; 10+ messages in thread
From: Vincenzo Frascino @ 2019-07-25 13:50 UTC (permalink / raw)
  To: linux-arm-kernel, linux-doc, linux-mm, linux-arch,
	linux-kselftest, linux-kernel
  Cc: vincenzo.frascino, Catalin Marinas, Will Deacon, Andrey Konovalov,
	Szabolcs Nagy

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 in this set, 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().

Relax the requirements described in tagged-pointers.rst to be compliant
with the behaviours guaranteed by the ARM64 Tagged Address ABI.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
CC: Andrey Konovalov <andreyknvl@google.com>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
Acked-by: Szabolcs Nagy <szabolcs.nagy@arm.com>
---
 Documentation/arm64/tagged-pointers.rst | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/Documentation/arm64/tagged-pointers.rst b/Documentation/arm64/tagged-pointers.rst
index 2acdec3ebbeb..933aaef8d52f 100644
--- a/Documentation/arm64/tagged-pointers.rst
+++ b/Documentation/arm64/tagged-pointers.rst
@@ -20,7 +20,8 @@ Passing tagged addresses to the kernel
 --------------------------------------
 
 All interpretation of userspace memory addresses by the kernel assumes
-an address tag of 0x00.
+an address tag of 0x00, unless the userspace opts-in the ARM64 Tagged
+Address ABI via the PR_SET_TAGGED_ADDR_CTRL prctl().
 
 This includes, but is not limited to, addresses found in:
 
@@ -33,18 +34,23 @@ This includes, but is not limited to, addresses found in:
  - the frame pointer (x29) and frame records, e.g. when interpreting
    them to generate a backtrace or call graph.
 
-Using non-zero address tags in any of these locations may result in an
-error code being returned, a (fatal) signal being raised, or other modes
-of failure.
+Using non-zero address tags in any of these locations when the
+userspace application did not opt-in to the ARM64 Tagged Address ABI
+may result in an error code being returned, a (fatal) signal being raised,
+or other modes of failure.
 
-For these reasons, passing non-zero address tags to the kernel via
-system calls is forbidden, and using a non-zero address tag for sp is
-strongly discouraged.
+For these reasons, when the userspace application did not opt-in, passing
+non-zero address tags to the kernel via system calls is forbidden, and using
+a non-zero address tag for sp is strongly discouraged.
 
 Programs maintaining a frame pointer and frame records that use non-zero
 address tags may suffer impaired or inaccurate debug and profiling
 visibility.
 
+A definition of the meaning of ARM64 Tagged Address ABI and of the
+guarantees that the ABI provides when the userspace opts-in via prctl()
+can be found in: Documentation/arm64/tagged-address-abi.rst.
+
 
 Preserving tags
 ---------------
@@ -59,6 +65,9 @@ be preserved.
 The architecture prevents the use of a tagged PC, so the upper byte will
 be set to a sign-extension of bit 55 on exception return.
 
+These behaviours are preserved even when the userspace opts-in to the ARM64
+Tagged Address ABI via the PR_SET_TAGGED_ADDR_CTRL prctl().
+
 
 Other considerations
 --------------------
-- 
2.22.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v6 1/2] arm64: Define Documentation/arm64/tagged-address-abi.rst
  2019-07-25 13:50   ` [PATCH v6 1/2] arm64: Define Documentation/arm64/tagged-address-abi.rst Vincenzo Frascino
@ 2019-07-30 10:32     ` Kevin Brodsky
  2019-07-30 13:25       ` Vincenzo Frascino
  2019-07-31 16:43     ` Dave Hansen
  1 sibling, 1 reply; 10+ messages in thread
From: Kevin Brodsky @ 2019-07-30 10:32 UTC (permalink / raw)
  To: Vincenzo Frascino, linux-arm-kernel, linux-doc, linux-mm,
	linux-arch, linux-kselftest, linux-kernel
  Cc: Szabolcs Nagy, Catalin Marinas, Will Deacon, Andrey Konovalov

Some more comments. Mostly minor wording issues, except the prctl() exclusion at the end.

On 25/07/2019 14:50, Vincenzo Frascino wrote:
> 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: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> CC: Andrey Konovalov <andreyknvl@google.com>
> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
> Acked-by: Szabolcs Nagy <szabolcs.nagy@arm.com>
> ---
>   Documentation/arm64/tagged-address-abi.rst | 148 +++++++++++++++++++++
>   1 file changed, 148 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..a8ecb991de82
> --- /dev/null
> +++ b/Documentation/arm64/tagged-address-abi.rst
> @@ -0,0 +1,148 @@
> +========================
> +ARM64 TAGGED ADDRESS ABI
> +========================
> +
> +Author: Vincenzo Frascino <vincenzo.frascino@arm.com>
> +
> +Date: 25 July 2019
> +
> +This document describes the usage and semantics of the Tagged Address
> +ABI on arm64.
> +
> +1. Introduction
> +---------------
> +
> +On arm64 the TCR_EL1.TBI0 bit has always been enabled on the kernel, hence
> +the userspace (EL0) is entitled to perform a user memory access through a
> +64-bit pointer with a non-zero top byte but the resulting pointers are not
> +allowed at the user-kernel syscall ABI boundary.
> +
> +This document describes a relaxation of the ABI that makes it possible to
> +to pass tagged pointers to the syscalls, when these pointers are in memory

One too many "to" (at the end the previous line).

> +ranges obtained as described in section 2.
> +
> +Since it is not desirable to relax the ABI to allow tagged user addresses
> +into the kernel indiscriminately, arm64 provides a new sysctl interface
> +(/proc/sys/abi/tagged_addr) that is used to prevent the applications from
> +enabling the relaxed ABI and a new prctl() interface that can be used to
> +enable or disable the relaxed ABI.
> +A detailed description of the newly introduced mechanisms will be provided
> +in section 2.
> +
> +2. ARM64 Tagged Address ABI
> +---------------------------
> +
> +From the kernel syscall interface perspective, we define, for the purposes
> +of this document, a "valid tagged pointer" as a pointer that either has a
> +zero value set in the top byte or has a non-zero value, is in memory ranges
> +privately owned by a userspace process and is obtained in one of the
> +following ways:
> +- mmap() done by the process itself, where either:
> +
> +  - flags have **MAP_PRIVATE** and **MAP_ANONYMOUS**
> +  - flags have **MAP_PRIVATE** and the file descriptor refers to a regular
> +    file 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 process's address space during
> +  creation and with the same restrictions as for mmap() (e.g. data, bss,
> +  stack).
> +
> +The ARM64 Tagged Address ABI is an opt-in feature, and an application can
> +control it using the following:
> +
> +- **/proc/sys/abi/tagged_addr**: a new sysctl interface that can be used to
> +  prevent the applications from enabling the access to the relaxed ABI.
> +  The sysctl supports the following configuration options:
> +
> +  - **0**: Disable the access to the ARM64 Tagged Address ABI for all
> +    the applications.
> +  - **1** (Default): Enable the access to the ARM64 Tagged Address ABI for
> +    all the applications.
> +
> +   If the access to the ARM64 Tagged Address ABI is disabled at a certain
> +   point in time, all the applications that were using tagging before this
> +   event occurs, will continue to use tagging.

"tagging" may be misinterpreted here. I would be more explicit by saying that the 
tagged address ABI remains enabled in processes that opted in before the access got 
disabled.

> +- **prctl()s**:
> +
> +  - **PR_SET_TAGGED_ADDR_CTRL**: Invoked by a process, can be used to enable or
> +    disable its access to the ARM64 Tagged Address ABI.

I still find the wording confusing, because "access to the ABI" is not used 
consistently. The "tagged_addr" sysctl enables *access to the ABI*, that's fine. 
However, PR_SET_TAGGED_ADDR_CTRL enables *the ABI itself* (which is only possible if 
access to the ABI is enabled).

> +
> +    The (unsigned int) arg2 argument is a bit mask describing the control mode
> +    used:
> +
> +    - **PR_TAGGED_ADDR_ENABLE**: Enable ARM64 Tagged Address ABI.
> +
> +    The prctl(PR_SET_TAGGED_ADDR_CTRL, ...) will return -EINVAL if the ARM64
> +    Tagged Address ABI is not available.

For clarity, it would be good to mention that one possible reason for the ABI not to 
be available is tagged_addr == 0.

> +
> +    The arguments arg3, arg4, and arg5 are ignored.
> +  - **PR_GET_TAGGED_ADDR_CTRL**: can be used to check the status of the Tagged
> +    Address ABI.
> +
> +    The arguments arg2, arg3, arg4, and arg5 are ignored.
> +
> +The ABI properties set by the mechanisms described above are inherited by threads
> +of the same application and fork()'ed children but cleared by execve().
> +
> +When a process has successfully opted into the new ABI by invoking
> +PR_SET_TAGGED_ADDR_CTRL prctl(), this guarantees the following behaviours:
> +
> + - 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.

I thought Catalin wanted to drop this guarantee?

> + - If a non valid tagged pointer is passed to a syscall then the behaviour
> +   is undefined.
> + - Every valid tagged pointer is expected to work as an untagged one.
> + - The kernel preserves any valid tagged pointer and returns it to the
> +   userspace unchanged (i.e. on syscall return) in all the cases except the
> +   ones documented in the "Preserving tags" section of tagged-pointers.txt.
> +
> +A definition of the meaning of tagged pointers on arm64 can be found in:
> +Documentation/arm64/tagged-pointers.txt.
> +
> +3. ARM64 Tagged Address ABI Exceptions
> +--------------------------------------
> +
> +The behaviours described in section 2, with particular reference to the
> +acceptance by the syscalls of any valid tagged pointer are not applicable
> +to the following cases:
> +
> + - mmap() addr parameter.
> + - mremap() new_address parameter.
> + - prctl(PR_SET_MM, PR_SET_MM_MAP, ...) struct prctl_mm_map fields.
> + - prctl(PR_SET_MM, PR_SET_MM_MAP_SIZE, ...) struct prctl_mm_map fields.

All the PR_SET_MM options that specify pointers (PR_SET_MM_START_CODE, 
PR_SET_MM_END_CODE, ...) should be excluded as well. AFAICT (but don't take my word 
for it), that's all of them except PR_SET_MM_EXE_FILE. Conversely, PR_SET_MM_MAP_SIZE 
should not be excluded (it does not pass a prctl_mm_map struct, and the pointer to 
unsigned int can be tagged).

Kevin

> +
> +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';
> +
> +           ...
> +   }
> +

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v6 1/2] arm64: Define Documentation/arm64/tagged-address-abi.rst
  2019-07-30 10:32     ` Kevin Brodsky
@ 2019-07-30 13:25       ` Vincenzo Frascino
  2019-07-30 13:57         ` Kevin Brodsky
  0 siblings, 1 reply; 10+ messages in thread
From: Vincenzo Frascino @ 2019-07-30 13:25 UTC (permalink / raw)
  To: Kevin Brodsky, linux-arm-kernel, linux-doc, linux-mm, linux-arch,
	linux-kselftest, linux-kernel
  Cc: Szabolcs Nagy, Catalin Marinas, Will Deacon, Andrey Konovalov

Hi Kevin,

On 7/30/19 11:32 AM, Kevin Brodsky wrote:
> Some more comments. Mostly minor wording issues, except the prctl() exclusion at
> the end.
> 
> On 25/07/2019 14:50, Vincenzo Frascino wrote:
>> 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: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Will Deacon <will.deacon@arm.com>
>> CC: Andrey Konovalov <andreyknvl@google.com>
>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
>> Acked-by: Szabolcs Nagy <szabolcs.nagy@arm.com>
>> ---
>>   Documentation/arm64/tagged-address-abi.rst | 148 +++++++++++++++++++++
>>   1 file changed, 148 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..a8ecb991de82
>> --- /dev/null
>> +++ b/Documentation/arm64/tagged-address-abi.rst
>> @@ -0,0 +1,148 @@
>> +========================
>> +ARM64 TAGGED ADDRESS ABI
>> +========================
>> +
>> +Author: Vincenzo Frascino <vincenzo.frascino@arm.com>
>> +
>> +Date: 25 July 2019
>> +
>> +This document describes the usage and semantics of the Tagged Address
>> +ABI on arm64.
>> +
>> +1. Introduction
>> +---------------
>> +
>> +On arm64 the TCR_EL1.TBI0 bit has always been enabled on the kernel, hence
>> +the userspace (EL0) is entitled to perform a user memory access through a
>> +64-bit pointer with a non-zero top byte but the resulting pointers are not
>> +allowed at the user-kernel syscall ABI boundary.
>> +
>> +This document describes a relaxation of the ABI that makes it possible to
>> +to pass tagged pointers to the syscalls, when these pointers are in memory
> 
> One too many "to" (at the end the previous line).
> 

Yep will fix in v7.

>> +ranges obtained as described in section 2.
>> +
>> +Since it is not desirable to relax the ABI to allow tagged user addresses
>> +into the kernel indiscriminately, arm64 provides a new sysctl interface
>> +(/proc/sys/abi/tagged_addr) that is used to prevent the applications from
>> +enabling the relaxed ABI and a new prctl() interface that can be used to
>> +enable or disable the relaxed ABI.
>> +A detailed description of the newly introduced mechanisms will be provided
>> +in section 2.
>> +
>> +2. ARM64 Tagged Address ABI
>> +---------------------------
>> +
>> +From the kernel syscall interface perspective, we define, for the purposes
>> +of this document, a "valid tagged pointer" as a pointer that either has a
>> +zero value set in the top byte or has a non-zero value, is in memory ranges
>> +privately owned by a userspace process and is obtained in one of the
>> +following ways:
>> +- mmap() done by the process itself, where either:
>> +
>> +  - flags have **MAP_PRIVATE** and **MAP_ANONYMOUS**
>> +  - flags have **MAP_PRIVATE** and the file descriptor refers to a regular
>> +    file 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 process's address space during
>> +  creation and with the same restrictions as for mmap() (e.g. data, bss,
>> +  stack).
>> +
>> +The ARM64 Tagged Address ABI is an opt-in feature, and an application can
>> +control it using the following:
>> +
>> +- **/proc/sys/abi/tagged_addr**: a new sysctl interface that can be used to
>> +  prevent the applications from enabling the access to the relaxed ABI.
>> +  The sysctl supports the following configuration options:
>> +
>> +  - **0**: Disable the access to the ARM64 Tagged Address ABI for all
>> +    the applications.
>> +  - **1** (Default): Enable the access to the ARM64 Tagged Address ABI for
>> +    all the applications.
>> +
>> +   If the access to the ARM64 Tagged Address ABI is disabled at a certain
>> +   point in time, all the applications that were using tagging before this
>> +   event occurs, will continue to use tagging.
> 
> "tagging" may be misinterpreted here. I would be more explicit by saying that
> the tagged address ABI remains enabled in processes that opted in before the
> access got disabled.
> 

Assuming that ARM64 Tagged Address ABI gives access to "tagging" and since it is
what this document is talking about, I do not see how it can be misinterpreted ;)

>> +- **prctl()s**:
>> +
>> +  - **PR_SET_TAGGED_ADDR_CTRL**: Invoked by a process, can be used to enable or
>> +    disable its access to the ARM64 Tagged Address ABI.
> 
> I still find the wording confusing, because "access to the ABI" is not used
> consistently. The "tagged_addr" sysctl enables *access to the ABI*, that's fine.
> However, PR_SET_TAGGED_ADDR_CTRL enables *the ABI itself* (which is only
> possible if access to the ABI is enabled).
> 

As it stands, it enables or disables the ABI itself when used with
PR_TAGGED_ADDR_ENABLE, or can enable other things in future. IMHO the only thing
that these features have in common is the access to the ABI which is granted by
this prctl().

>> +
>> +    The (unsigned int) arg2 argument is a bit mask describing the control mode
>> +    used:
>> +
>> +    - **PR_TAGGED_ADDR_ENABLE**: Enable ARM64 Tagged Address ABI.
>> +
>> +    The prctl(PR_SET_TAGGED_ADDR_CTRL, ...) will return -EINVAL if the ARM64
>> +    Tagged Address ABI is not available.
> 
> For clarity, it would be good to mention that one possible reason for the ABI
> not to be available is tagged_addr == 0.
> 

The logical implication is already quite clear tagged_addr == 0 (Disabled) =>
Tagged Address ABI not available => return -EINVAL. I do not see the need to
repeat the concept twice.

>> +
>> +    The arguments arg3, arg4, and arg5 are ignored.
>> +  - **PR_GET_TAGGED_ADDR_CTRL**: can be used to check the status of the Tagged
>> +    Address ABI.
>> +
>> +    The arguments arg2, arg3, arg4, and arg5 are ignored.
>> +
>> +The ABI properties set by the mechanisms described above are inherited by
>> threads
>> +of the same application and fork()'ed children but cleared by execve().
>> +
>> +When a process has successfully opted into the new ABI by invoking
>> +PR_SET_TAGGED_ADDR_CTRL prctl(), this guarantees the following behaviours:
>> +
>> + - 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.
> 
> I thought Catalin wanted to drop this guarantee?
> 

The guarantee is changed and explicitly includes the syscalls that can be added
in the future. IMHO since we are defining an ABI, we cannot leave that topic in
an uncharted territory, we need to address it.

>> + - If a non valid tagged pointer is passed to a syscall then the behaviour
>> +   is undefined.
>> + - Every valid tagged pointer is expected to work as an untagged one.
>> + - The kernel preserves any valid tagged pointer and returns it to the
>> +   userspace unchanged (i.e. on syscall return) in all the cases except the
>> +   ones documented in the "Preserving tags" section of tagged-pointers.txt.
>> +
>> +A definition of the meaning of tagged pointers on arm64 can be found in:
>> +Documentation/arm64/tagged-pointers.txt.
>> +
>> +3. ARM64 Tagged Address ABI Exceptions
>> +--------------------------------------
>> +
>> +The behaviours described in section 2, with particular reference to the
>> +acceptance by the syscalls of any valid tagged pointer are not applicable
>> +to the following cases:
>> +
>> + - mmap() addr parameter.
>> + - mremap() new_address parameter.
>> + - prctl(PR_SET_MM, PR_SET_MM_MAP, ...) struct prctl_mm_map fields.
>> + - prctl(PR_SET_MM, PR_SET_MM_MAP_SIZE, ...) struct prctl_mm_map fields.
> 
> All the PR_SET_MM options that specify pointers (PR_SET_MM_START_CODE,
> PR_SET_MM_END_CODE, ...) should be excluded as well. AFAICT (but don't take my
> word for it), that's all of them except PR_SET_MM_EXE_FILE. Conversely,
> PR_SET_MM_MAP_SIZE should not be excluded (it does not pass a prctl_mm_map
> struct, and the pointer to unsigned int can be tagged).
> 

Agreed, I clearly misread the prctl() man page here. Fill fix in v7.
PR_SET_MM_MAP_SIZE _returns_  struct prctl_mm_map, does not take it as a parameter.

Vincenzo

> Kevin
> 
>> +
>> +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';
>> +
>> +           ...
>> +   }
>> +
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
Regards,
Vincenzo

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v6 1/2] arm64: Define Documentation/arm64/tagged-address-abi.rst
  2019-07-30 13:25       ` Vincenzo Frascino
@ 2019-07-30 13:57         ` Kevin Brodsky
  2019-07-30 14:24           ` Vincenzo Frascino
  0 siblings, 1 reply; 10+ messages in thread
From: Kevin Brodsky @ 2019-07-30 13:57 UTC (permalink / raw)
  To: Vincenzo Frascino, linux-arm-kernel, linux-doc, linux-mm,
	linux-arch, linux-kselftest, linux-kernel
  Cc: Szabolcs Nagy, Catalin Marinas, Will Deacon, Andrey Konovalov

On 30/07/2019 14:25, Vincenzo Frascino wrote:
> Hi Kevin,
>
> On 7/30/19 11:32 AM, Kevin Brodsky wrote:
>> Some more comments. Mostly minor wording issues, except the prctl() exclusion at
>> the end.
>>
>> On 25/07/2019 14:50, Vincenzo Frascino wrote:
>>> 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: Catalin Marinas <catalin.marinas@arm.com>
>>> Cc: Will Deacon <will.deacon@arm.com>
>>> CC: Andrey Konovalov <andreyknvl@google.com>
>>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
>>> Acked-by: Szabolcs Nagy <szabolcs.nagy@arm.com>
>>> ---
>>>    Documentation/arm64/tagged-address-abi.rst | 148 +++++++++++++++++++++
>>>    1 file changed, 148 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..a8ecb991de82
>>> --- /dev/null
>>> +++ b/Documentation/arm64/tagged-address-abi.rst
>>> @@ -0,0 +1,148 @@
>>> +========================
>>> +ARM64 TAGGED ADDRESS ABI
>>> +========================
>>> +
>>> +Author: Vincenzo Frascino <vincenzo.frascino@arm.com>
>>> +
>>> +Date: 25 July 2019
>>> +
>>> +This document describes the usage and semantics of the Tagged Address
>>> +ABI on arm64.
>>> +
>>> +1. Introduction
>>> +---------------
>>> +
>>> +On arm64 the TCR_EL1.TBI0 bit has always been enabled on the kernel, hence
>>> +the userspace (EL0) is entitled to perform a user memory access through a
>>> +64-bit pointer with a non-zero top byte but the resulting pointers are not
>>> +allowed at the user-kernel syscall ABI boundary.
>>> +
>>> +This document describes a relaxation of the ABI that makes it possible to
>>> +to pass tagged pointers to the syscalls, when these pointers are in memory
>> One too many "to" (at the end the previous line).
>>
> Yep will fix in v7.
>
>>> +ranges obtained as described in section 2.
>>> +
>>> +Since it is not desirable to relax the ABI to allow tagged user addresses
>>> +into the kernel indiscriminately, arm64 provides a new sysctl interface
>>> +(/proc/sys/abi/tagged_addr) that is used to prevent the applications from
>>> +enabling the relaxed ABI and a new prctl() interface that can be used to
>>> +enable or disable the relaxed ABI.
>>> +A detailed description of the newly introduced mechanisms will be provided
>>> +in section 2.
>>> +
>>> +2. ARM64 Tagged Address ABI
>>> +---------------------------
>>> +
>>> +From the kernel syscall interface perspective, we define, for the purposes
>>> +of this document, a "valid tagged pointer" as a pointer that either has a
>>> +zero value set in the top byte or has a non-zero value, is in memory ranges
>>> +privately owned by a userspace process and is obtained in one of the
>>> +following ways:
>>> +- mmap() done by the process itself, where either:
>>> +
>>> +  - flags have **MAP_PRIVATE** and **MAP_ANONYMOUS**
>>> +  - flags have **MAP_PRIVATE** and the file descriptor refers to a regular
>>> +    file 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 process's address space during
>>> +  creation and with the same restrictions as for mmap() (e.g. data, bss,
>>> +  stack).
>>> +
>>> +The ARM64 Tagged Address ABI is an opt-in feature, and an application can
>>> +control it using the following:
>>> +
>>> +- **/proc/sys/abi/tagged_addr**: a new sysctl interface that can be used to
>>> +  prevent the applications from enabling the access to the relaxed ABI.
>>> +  The sysctl supports the following configuration options:
>>> +
>>> +  - **0**: Disable the access to the ARM64 Tagged Address ABI for all
>>> +    the applications.
>>> +  - **1** (Default): Enable the access to the ARM64 Tagged Address ABI for
>>> +    all the applications.
>>> +
>>> +   If the access to the ARM64 Tagged Address ABI is disabled at a certain
>>> +   point in time, all the applications that were using tagging before this
>>> +   event occurs, will continue to use tagging.
>> "tagging" may be misinterpreted here. I would be more explicit by saying that
>> the tagged address ABI remains enabled in processes that opted in before the
>> access got disabled.
>>
> Assuming that ARM64 Tagged Address ABI gives access to "tagging" and since it is
> what this document is talking about, I do not see how it can be misinterpreted ;)

"tagging" is a confusing term ("using tagging" even more so), it could be interpreted 
as memory tagging (especially in the presence of MTE). This document does not use 
"tagging" anywhere else, which is good. Let's stick to the same name for the ABI 
throughout the document, repetition is less problematic than vague wording.

>
>>> +- **prctl()s**:
>>> +
>>> +  - **PR_SET_TAGGED_ADDR_CTRL**: Invoked by a process, can be used to enable or
>>> +    disable its access to the ARM64 Tagged Address ABI.
>> I still find the wording confusing, because "access to the ABI" is not used
>> consistently. The "tagged_addr" sysctl enables *access to the ABI*, that's fine.
>> However, PR_SET_TAGGED_ADDR_CTRL enables *the ABI itself* (which is only
>> possible if access to the ABI is enabled).
>>
> As it stands, it enables or disables the ABI itself when used with
> PR_TAGGED_ADDR_ENABLE, or can enable other things in future. IMHO the only thing
> that these features have in common is the access to the ABI which is granted by
> this prctl().

I see your point, you could have other bits controlling other aspects. However, I 
would really avoid saying that this prctl is used to enable or disable access to the 
new ABI, because it isn't (either you have access to the new ABI and this prctl can 
be used, or you don't and this prctl will fail).

>
>>> +
>>> +    The (unsigned int) arg2 argument is a bit mask describing the control mode
>>> +    used:
>>> +
>>> +    - **PR_TAGGED_ADDR_ENABLE**: Enable ARM64 Tagged Address ABI.
>>> +
>>> +    The prctl(PR_SET_TAGGED_ADDR_CTRL, ...) will return -EINVAL if the ARM64
>>> +    Tagged Address ABI is not available.
>> For clarity, it would be good to mention that one possible reason for the ABI
>> not to be available is tagged_addr == 0.
>>
> The logical implication is already quite clear tagged_addr == 0 (Disabled) =>
> Tagged Address ABI not available => return -EINVAL. I do not see the need to
> repeat the concept twice.
>
>>> +
>>> +    The arguments arg3, arg4, and arg5 are ignored.
>>> +  - **PR_GET_TAGGED_ADDR_CTRL**: can be used to check the status of the Tagged
>>> +    Address ABI.
>>> +
>>> +    The arguments arg2, arg3, arg4, and arg5 are ignored.
>>> +
>>> +The ABI properties set by the mechanisms described above are inherited by
>>> threads
>>> +of the same application and fork()'ed children but cleared by execve().
>>> +
>>> +When a process has successfully opted into the new ABI by invoking
>>> +PR_SET_TAGGED_ADDR_CTRL prctl(), this guarantees the following behaviours:
>>> +
>>> + - 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.
>> I thought Catalin wanted to drop this guarantee?
>>
> The guarantee is changed and explicitly includes the syscalls that can be added
> in the future. IMHO since we are defining an ABI, we cannot leave that topic in
> an uncharted territory, we need to address it.

It makes sense to me, just wanted to be sure that Catalin is on the same page.

>
>>> + - If a non valid tagged pointer is passed to a syscall then the behaviour
>>> +   is undefined.
>>> + - Every valid tagged pointer is expected to work as an untagged one.
>>> + - The kernel preserves any valid tagged pointer and returns it to the
>>> +   userspace unchanged (i.e. on syscall return) in all the cases except the
>>> +   ones documented in the "Preserving tags" section of tagged-pointers.txt.
>>> +
>>> +A definition of the meaning of tagged pointers on arm64 can be found in:
>>> +Documentation/arm64/tagged-pointers.txt.
>>> +
>>> +3. ARM64 Tagged Address ABI Exceptions
>>> +--------------------------------------
>>> +
>>> +The behaviours described in section 2, with particular reference to the
>>> +acceptance by the syscalls of any valid tagged pointer are not applicable
>>> +to the following cases:
>>> +
>>> + - mmap() addr parameter.
>>> + - mremap() new_address parameter.
>>> + - prctl(PR_SET_MM, PR_SET_MM_MAP, ...) struct prctl_mm_map fields.
>>> + - prctl(PR_SET_MM, PR_SET_MM_MAP_SIZE, ...) struct prctl_mm_map fields.
>> All the PR_SET_MM options that specify pointers (PR_SET_MM_START_CODE,
>> PR_SET_MM_END_CODE, ...) should be excluded as well. AFAICT (but don't take my
>> word for it), that's all of them except PR_SET_MM_EXE_FILE. Conversely,
>> PR_SET_MM_MAP_SIZE should not be excluded (it does not pass a prctl_mm_map
>> struct, and the pointer to unsigned int can be tagged).
>>
> Agreed, I clearly misread the prctl() man page here. Fill fix in v7.
> PR_SET_MM_MAP_SIZE _returns_  struct prctl_mm_map, does not take it as a parameter.

OK. About PR_SET_MM_MAP_SIZE, it neither takes nor returns struct prctl_mm_map. It 
writes the size of prctl_map to the int pointed to by arg3, and does nothing else. 
Therefore, there's no need to exclude it.

BTW I've just realised that the man page is wrong about PR_SET_MM_MAP_SIZE, the 
pointer to int is passed in arg3, not arg4. Anyone knows where to report that?

Thanks,
Kevin

> Vincenzo
>
>> Kevin
>>
>>> +
>>> +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';
>>> +
>>> +           ...
>>> +   }
>>> +
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v6 1/2] arm64: Define Documentation/arm64/tagged-address-abi.rst
  2019-07-30 13:57         ` Kevin Brodsky
@ 2019-07-30 14:24           ` Vincenzo Frascino
  2019-07-30 14:48             ` Kevin Brodsky
  0 siblings, 1 reply; 10+ messages in thread
From: Vincenzo Frascino @ 2019-07-30 14:24 UTC (permalink / raw)
  To: Kevin Brodsky, linux-arm-kernel, linux-doc, linux-mm, linux-arch,
	linux-kselftest, linux-kernel
  Cc: Szabolcs Nagy, Catalin Marinas, Will Deacon, Andrey Konovalov

Hi Kevin,

On 7/30/19 2:57 PM, Kevin Brodsky wrote:
> On 30/07/2019 14:25, Vincenzo Frascino wrote:
>> Hi Kevin,
>>
>> On 7/30/19 11:32 AM, Kevin Brodsky wrote:
>>> Some more comments. Mostly minor wording issues, except the prctl() exclusion at
>>> the end.
>>>
>>> On 25/07/2019 14:50, Vincenzo Frascino wrote:
>>>> 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: Catalin Marinas <catalin.marinas@arm.com>
>>>> Cc: Will Deacon <will.deacon@arm.com>
>>>> CC: Andrey Konovalov <andreyknvl@google.com>
>>>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
>>>> Acked-by: Szabolcs Nagy <szabolcs.nagy@arm.com>
>>>> ---
>>>>    Documentation/arm64/tagged-address-abi.rst | 148 +++++++++++++++++++++
>>>>    1 file changed, 148 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..a8ecb991de82
>>>> --- /dev/null
>>>> +++ b/Documentation/arm64/tagged-address-abi.rst
>>>> @@ -0,0 +1,148 @@
>>>> +========================
>>>> +ARM64 TAGGED ADDRESS ABI
>>>> +========================
>>>> +
>>>> +Author: Vincenzo Frascino <vincenzo.frascino@arm.com>
>>>> +
>>>> +Date: 25 July 2019
>>>> +
>>>> +This document describes the usage and semantics of the Tagged Address
>>>> +ABI on arm64.
>>>> +
>>>> +1. Introduction
>>>> +---------------
>>>> +
>>>> +On arm64 the TCR_EL1.TBI0 bit has always been enabled on the kernel, hence
>>>> +the userspace (EL0) is entitled to perform a user memory access through a
>>>> +64-bit pointer with a non-zero top byte but the resulting pointers are not
>>>> +allowed at the user-kernel syscall ABI boundary.
>>>> +
>>>> +This document describes a relaxation of the ABI that makes it possible to
>>>> +to pass tagged pointers to the syscalls, when these pointers are in memory
>>> One too many "to" (at the end the previous line).
>>>
>> Yep will fix in v7.
>>
>>>> +ranges obtained as described in section 2.
>>>> +
>>>> +Since it is not desirable to relax the ABI to allow tagged user addresses
>>>> +into the kernel indiscriminately, arm64 provides a new sysctl interface
>>>> +(/proc/sys/abi/tagged_addr) that is used to prevent the applications from
>>>> +enabling the relaxed ABI and a new prctl() interface that can be used to
>>>> +enable or disable the relaxed ABI.
>>>> +A detailed description of the newly introduced mechanisms will be provided
>>>> +in section 2.
>>>> +
>>>> +2. ARM64 Tagged Address ABI
>>>> +---------------------------
>>>> +
>>>> +From the kernel syscall interface perspective, we define, for the purposes
>>>> +of this document, a "valid tagged pointer" as a pointer that either has a
>>>> +zero value set in the top byte or has a non-zero value, is in memory ranges
>>>> +privately owned by a userspace process and is obtained in one of the
>>>> +following ways:
>>>> +- mmap() done by the process itself, where either:
>>>> +
>>>> +  - flags have **MAP_PRIVATE** and **MAP_ANONYMOUS**
>>>> +  - flags have **MAP_PRIVATE** and the file descriptor refers to a regular
>>>> +    file 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 process's address space during
>>>> +  creation and with the same restrictions as for mmap() (e.g. data, bss,
>>>> +  stack).
>>>> +
>>>> +The ARM64 Tagged Address ABI is an opt-in feature, and an application can
>>>> +control it using the following:
>>>> +
>>>> +- **/proc/sys/abi/tagged_addr**: a new sysctl interface that can be used to
>>>> +  prevent the applications from enabling the access to the relaxed ABI.
>>>> +  The sysctl supports the following configuration options:
>>>> +
>>>> +  - **0**: Disable the access to the ARM64 Tagged Address ABI for all
>>>> +    the applications.
>>>> +  - **1** (Default): Enable the access to the ARM64 Tagged Address ABI for
>>>> +    all the applications.
>>>> +
>>>> +   If the access to the ARM64 Tagged Address ABI is disabled at a certain
>>>> +   point in time, all the applications that were using tagging before this
>>>> +   event occurs, will continue to use tagging.
>>> "tagging" may be misinterpreted here. I would be more explicit by saying that
>>> the tagged address ABI remains enabled in processes that opted in before the
>>> access got disabled.
>>>
>> Assuming that ARM64 Tagged Address ABI gives access to "tagging" and since it is
>> what this document is talking about, I do not see how it can be misinterpreted ;)
> 
> "tagging" is a confusing term ("using tagging" even more so), it could be
> interpreted as memory tagging (especially in the presence of MTE). This document
> does not use "tagging" anywhere else, which is good. Let's stick to the same
> name for the ABI throughout the document, repetition is less problematic than
> vague wording.
> 

This document does not cover MTE, it covers the "ARM64 Tagged Address ABI" hence
"tagging" has a precise semantical meaning in this context. Still I do not see
how it can be confused.

>>
>>>> +- **prctl()s**:
>>>> +
>>>> +  - **PR_SET_TAGGED_ADDR_CTRL**: Invoked by a process, can be used to
>>>> enable or
>>>> +    disable its access to the ARM64 Tagged Address ABI.
>>> I still find the wording confusing, because "access to the ABI" is not used
>>> consistently. The "tagged_addr" sysctl enables *access to the ABI*, that's fine.
>>> However, PR_SET_TAGGED_ADDR_CTRL enables *the ABI itself* (which is only
>>> possible if access to the ABI is enabled).
>>>
>> As it stands, it enables or disables the ABI itself when used with
>> PR_TAGGED_ADDR_ENABLE, or can enable other things in future. IMHO the only thing
>> that these features have in common is the access to the ABI which is granted by
>> this prctl().
> 
> I see your point, you could have other bits controlling other aspects. However,
> I would really avoid saying that this prctl is used to enable or disable access
> to the new ABI, because it isn't (either you have access to the new ABI and this
> prctl can be used, or you don't and this prctl will fail).
> 

What is the system wide evidence that the access to the ABI is denied? Or what
is the system wide evidence that it is granted?

In other words, is it enough for a process to have the sysctl set (system wide)
to know that the the ABI is enabled and have granted access to it? or does it
need to do something else?

>>
>>>> +
>>>> +    The (unsigned int) arg2 argument is a bit mask describing the control mode
>>>> +    used:
>>>> +
>>>> +    - **PR_TAGGED_ADDR_ENABLE**: Enable ARM64 Tagged Address ABI.
>>>> +
>>>> +    The prctl(PR_SET_TAGGED_ADDR_CTRL, ...) will return -EINVAL if the ARM64
>>>> +    Tagged Address ABI is not available.
>>> For clarity, it would be good to mention that one possible reason for the ABI
>>> not to be available is tagged_addr == 0.
>>>
>> The logical implication is already quite clear tagged_addr == 0 (Disabled) =>
>> Tagged Address ABI not available => return -EINVAL. I do not see the need to
>> repeat the concept twice.
>>
>>>> +
>>>> +    The arguments arg3, arg4, and arg5 are ignored.
>>>> +  - **PR_GET_TAGGED_ADDR_CTRL**: can be used to check the status of the Tagged
>>>> +    Address ABI.
>>>> +
>>>> +    The arguments arg2, arg3, arg4, and arg5 are ignored.
>>>> +
>>>> +The ABI properties set by the mechanisms described above are inherited by
>>>> threads
>>>> +of the same application and fork()'ed children but cleared by execve().
>>>> +
>>>> +When a process has successfully opted into the new ABI by invoking
>>>> +PR_SET_TAGGED_ADDR_CTRL prctl(), this guarantees the following behaviours:
>>>> +
>>>> + - 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.
>>> I thought Catalin wanted to drop this guarantee?
>>>
>> The guarantee is changed and explicitly includes the syscalls that can be added
>> in the future. IMHO since we are defining an ABI, we cannot leave that topic in
>> an uncharted territory, we need to address it.
> 
> It makes sense to me, just wanted to be sure that Catalin is on the same page.
> 
>>
>>>> + - If a non valid tagged pointer is passed to a syscall then the behaviour
>>>> +   is undefined.
>>>> + - Every valid tagged pointer is expected to work as an untagged one.
>>>> + - The kernel preserves any valid tagged pointer and returns it to the
>>>> +   userspace unchanged (i.e. on syscall return) in all the cases except the
>>>> +   ones documented in the "Preserving tags" section of tagged-pointers.txt.
>>>> +
>>>> +A definition of the meaning of tagged pointers on arm64 can be found in:
>>>> +Documentation/arm64/tagged-pointers.txt.
>>>> +
>>>> +3. ARM64 Tagged Address ABI Exceptions
>>>> +--------------------------------------
>>>> +
>>>> +The behaviours described in section 2, with particular reference to the
>>>> +acceptance by the syscalls of any valid tagged pointer are not applicable
>>>> +to the following cases:
>>>> +
>>>> + - mmap() addr parameter.
>>>> + - mremap() new_address parameter.
>>>> + - prctl(PR_SET_MM, PR_SET_MM_MAP, ...) struct prctl_mm_map fields.
>>>> + - prctl(PR_SET_MM, PR_SET_MM_MAP_SIZE, ...) struct prctl_mm_map fields.
>>> All the PR_SET_MM options that specify pointers (PR_SET_MM_START_CODE,
>>> PR_SET_MM_END_CODE, ...) should be excluded as well. AFAICT (but don't take my
>>> word for it), that's all of them except PR_SET_MM_EXE_FILE. Conversely,
>>> PR_SET_MM_MAP_SIZE should not be excluded (it does not pass a prctl_mm_map
>>> struct, and the pointer to unsigned int can be tagged).
>>>
>> Agreed, I clearly misread the prctl() man page here. Fill fix in v7.
>> PR_SET_MM_MAP_SIZE _returns_  struct prctl_mm_map, does not take it as a
>> parameter.
> 
> OK. About PR_SET_MM_MAP_SIZE, it neither takes nor returns struct prctl_mm_map.
> It writes the size of prctl_map to the int pointed to by arg3, and does nothing
> else. Therefore, there's no need to exclude it.
> 

Agreed, I missed the word size in my reply: s/_returns_  struct
prctl_mm_map/_returns_  the size of struct prctl_mm_map/

> BTW I've just realised that the man page is wrong about PR_SET_MM_MAP_SIZE, the
> pointer to int is passed in arg3, not arg4. Anyone knows where to report that?
> 
> Thanks,
> Kevin
> 
>> Vincenzo
>>
>>> Kevin
>>>
>>>> +
>>>> +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';
>>>> +
>>>> +           ...
>>>> +   }
>>>> +
>>> _______________________________________________
>>> linux-arm-kernel mailing list
>>> linux-arm-kernel@lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

-- 
Regards,
Vincenzo

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v6 1/2] arm64: Define Documentation/arm64/tagged-address-abi.rst
  2019-07-30 14:24           ` Vincenzo Frascino
@ 2019-07-30 14:48             ` Kevin Brodsky
  0 siblings, 0 replies; 10+ messages in thread
From: Kevin Brodsky @ 2019-07-30 14:48 UTC (permalink / raw)
  To: Vincenzo Frascino, linux-arm-kernel, linux-doc, linux-mm,
	linux-arch, linux-kselftest, linux-kernel
  Cc: Szabolcs Nagy, Catalin Marinas, Will Deacon, Andrey Konovalov

On 30/07/2019 15:24, Vincenzo Frascino wrote:
> Hi Kevin,
>
> On 7/30/19 2:57 PM, Kevin Brodsky wrote:
>> On 30/07/2019 14:25, Vincenzo Frascino wrote:
>>> Hi Kevin,
>>>
>>> On 7/30/19 11:32 AM, Kevin Brodsky wrote:
>>>> Some more comments. Mostly minor wording issues, except the prctl() exclusion at
>>>> the end.
>>>>
>>>> On 25/07/2019 14:50, Vincenzo Frascino wrote:
>>>>> 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: Catalin Marinas <catalin.marinas@arm.com>
>>>>> Cc: Will Deacon <will.deacon@arm.com>
>>>>> CC: Andrey Konovalov <andreyknvl@google.com>
>>>>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
>>>>> Acked-by: Szabolcs Nagy <szabolcs.nagy@arm.com>
>>>>> ---
>>>>>     Documentation/arm64/tagged-address-abi.rst | 148 +++++++++++++++++++++
>>>>>     1 file changed, 148 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..a8ecb991de82
>>>>> --- /dev/null
>>>>> +++ b/Documentation/arm64/tagged-address-abi.rst
>>>>> @@ -0,0 +1,148 @@
>>>>> +========================
>>>>> +ARM64 TAGGED ADDRESS ABI
>>>>> +========================
>>>>> +
>>>>> +Author: Vincenzo Frascino <vincenzo.frascino@arm.com>
>>>>> +
>>>>> +Date: 25 July 2019
>>>>> +
>>>>> +This document describes the usage and semantics of the Tagged Address
>>>>> +ABI on arm64.
>>>>> +
>>>>> +1. Introduction
>>>>> +---------------
>>>>> +
>>>>> +On arm64 the TCR_EL1.TBI0 bit has always been enabled on the kernel, hence
>>>>> +the userspace (EL0) is entitled to perform a user memory access through a
>>>>> +64-bit pointer with a non-zero top byte but the resulting pointers are not
>>>>> +allowed at the user-kernel syscall ABI boundary.
>>>>> +
>>>>> +This document describes a relaxation of the ABI that makes it possible to
>>>>> +to pass tagged pointers to the syscalls, when these pointers are in memory
>>>> One too many "to" (at the end the previous line).
>>>>
>>> Yep will fix in v7.
>>>
>>>>> +ranges obtained as described in section 2.
>>>>> +
>>>>> +Since it is not desirable to relax the ABI to allow tagged user addresses
>>>>> +into the kernel indiscriminately, arm64 provides a new sysctl interface
>>>>> +(/proc/sys/abi/tagged_addr) that is used to prevent the applications from
>>>>> +enabling the relaxed ABI and a new prctl() interface that can be used to
>>>>> +enable or disable the relaxed ABI.
>>>>> +A detailed description of the newly introduced mechanisms will be provided
>>>>> +in section 2.
>>>>> +
>>>>> +2. ARM64 Tagged Address ABI
>>>>> +---------------------------
>>>>> +
>>>>> +From the kernel syscall interface perspective, we define, for the purposes
>>>>> +of this document, a "valid tagged pointer" as a pointer that either has a
>>>>> +zero value set in the top byte or has a non-zero value, is in memory ranges
>>>>> +privately owned by a userspace process and is obtained in one of the
>>>>> +following ways:
>>>>> +- mmap() done by the process itself, where either:
>>>>> +
>>>>> +  - flags have **MAP_PRIVATE** and **MAP_ANONYMOUS**
>>>>> +  - flags have **MAP_PRIVATE** and the file descriptor refers to a regular
>>>>> +    file 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 process's address space during
>>>>> +  creation and with the same restrictions as for mmap() (e.g. data, bss,
>>>>> +  stack).
>>>>> +
>>>>> +The ARM64 Tagged Address ABI is an opt-in feature, and an application can
>>>>> +control it using the following:
>>>>> +
>>>>> +- **/proc/sys/abi/tagged_addr**: a new sysctl interface that can be used to
>>>>> +  prevent the applications from enabling the access to the relaxed ABI.
>>>>> +  The sysctl supports the following configuration options:
>>>>> +
>>>>> +  - **0**: Disable the access to the ARM64 Tagged Address ABI for all
>>>>> +    the applications.
>>>>> +  - **1** (Default): Enable the access to the ARM64 Tagged Address ABI for
>>>>> +    all the applications.
>>>>> +
>>>>> +   If the access to the ARM64 Tagged Address ABI is disabled at a certain
>>>>> +   point in time, all the applications that were using tagging before this
>>>>> +   event occurs, will continue to use tagging.
>>>> "tagging" may be misinterpreted here. I would be more explicit by saying that
>>>> the tagged address ABI remains enabled in processes that opted in before the
>>>> access got disabled.
>>>>
>>> Assuming that ARM64 Tagged Address ABI gives access to "tagging" and since it is
>>> what this document is talking about, I do not see how it can be misinterpreted ;)
>> "tagging" is a confusing term ("using tagging" even more so), it could be
>> interpreted as memory tagging (especially in the presence of MTE). This document
>> does not use "tagging" anywhere else, which is good. Let's stick to the same
>> name for the ABI throughout the document, repetition is less problematic than
>> vague wording.
>>
> This document does not cover MTE, it covers the "ARM64 Tagged Address ABI" hence
> "tagging" has a precise semantical meaning in this context. Still I do not see
> how it can be confused.
>
>>>>> +- **prctl()s**:
>>>>> +
>>>>> +  - **PR_SET_TAGGED_ADDR_CTRL**: Invoked by a process, can be used to
>>>>> enable or
>>>>> +    disable its access to the ARM64 Tagged Address ABI.
>>>> I still find the wording confusing, because "access to the ABI" is not used
>>>> consistently. The "tagged_addr" sysctl enables *access to the ABI*, that's fine.
>>>> However, PR_SET_TAGGED_ADDR_CTRL enables *the ABI itself* (which is only
>>>> possible if access to the ABI is enabled).
>>>>
>>> As it stands, it enables or disables the ABI itself when used with
>>> PR_TAGGED_ADDR_ENABLE, or can enable other things in future. IMHO the only thing
>>> that these features have in common is the access to the ABI which is granted by
>>> this prctl().
>> I see your point, you could have other bits controlling other aspects. However,
>> I would really avoid saying that this prctl is used to enable or disable access
>> to the new ABI, because it isn't (either you have access to the new ABI and this
>> prctl can be used, or you don't and this prctl will fail).
>>
> What is the system wide evidence that the access to the ABI is denied? Or what
> is the system wide evidence that it is granted?
>
> In other words, is it enough for a process to have the sysctl set (system wide)
> to know that the the ABI is enabled and have granted access to it? or does it
> need to do something else?

I think we really have a wording problem here, which is why this part of the document 
and this discussion is confusing.

tagged_addr=1 (system-wide) allows processes to enable the tagged address ABI by 
calling prctl(PR_SET_TAGGED_ADDR_CTRL). It does not alter the state of any running 
process, and does not enable the ABI by default for new processes either. Conversely, 
when tagged_addr=0, that prctl() is always denied.

The current description of the sysctl and prctl does not make that clear. I think 
that it would be much more obvious by reorganising that section as such:
- prctl() first, the current wording is fine.
- sysctl() second, described *only* in terms of the prctl() (denying 
PR_SET_TAGGED_ADDR_CTRL or not), and nothing else, to avoid wording issues.

It's certainly not the only way to do it, but that would be much clearer to me :)

Kevin

>>>>> +
>>>>> +    The (unsigned int) arg2 argument is a bit mask describing the control mode
>>>>> +    used:
>>>>> +
>>>>> +    - **PR_TAGGED_ADDR_ENABLE**: Enable ARM64 Tagged Address ABI.
>>>>> +
>>>>> +    The prctl(PR_SET_TAGGED_ADDR_CTRL, ...) will return -EINVAL if the ARM64
>>>>> +    Tagged Address ABI is not available.
>>>> For clarity, it would be good to mention that one possible reason for the ABI
>>>> not to be available is tagged_addr == 0.
>>>>
>>> The logical implication is already quite clear tagged_addr == 0 (Disabled) =>
>>> Tagged Address ABI not available => return -EINVAL. I do not see the need to
>>> repeat the concept twice.
>>>
>>>>> +
>>>>> +    The arguments arg3, arg4, and arg5 are ignored.
>>>>> +  - **PR_GET_TAGGED_ADDR_CTRL**: can be used to check the status of the Tagged
>>>>> +    Address ABI.
>>>>> +
>>>>> +    The arguments arg2, arg3, arg4, and arg5 are ignored.
>>>>> +
>>>>> +The ABI properties set by the mechanisms described above are inherited by
>>>>> threads
>>>>> +of the same application and fork()'ed children but cleared by execve().
>>>>> +
>>>>> +When a process has successfully opted into the new ABI by invoking
>>>>> +PR_SET_TAGGED_ADDR_CTRL prctl(), this guarantees the following behaviours:
>>>>> +
>>>>> + - 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.
>>>> I thought Catalin wanted to drop this guarantee?
>>>>
>>> The guarantee is changed and explicitly includes the syscalls that can be added
>>> in the future. IMHO since we are defining an ABI, we cannot leave that topic in
>>> an uncharted territory, we need to address it.
>> It makes sense to me, just wanted to be sure that Catalin is on the same page.
>>
>>>>> + - If a non valid tagged pointer is passed to a syscall then the behaviour
>>>>> +   is undefined.
>>>>> + - Every valid tagged pointer is expected to work as an untagged one.
>>>>> + - The kernel preserves any valid tagged pointer and returns it to the
>>>>> +   userspace unchanged (i.e. on syscall return) in all the cases except the
>>>>> +   ones documented in the "Preserving tags" section of tagged-pointers.txt.
>>>>> +
>>>>> +A definition of the meaning of tagged pointers on arm64 can be found in:
>>>>> +Documentation/arm64/tagged-pointers.txt.
>>>>> +
>>>>> +3. ARM64 Tagged Address ABI Exceptions
>>>>> +--------------------------------------
>>>>> +
>>>>> +The behaviours described in section 2, with particular reference to the
>>>>> +acceptance by the syscalls of any valid tagged pointer are not applicable
>>>>> +to the following cases:
>>>>> +
>>>>> + - mmap() addr parameter.
>>>>> + - mremap() new_address parameter.
>>>>> + - prctl(PR_SET_MM, PR_SET_MM_MAP, ...) struct prctl_mm_map fields.
>>>>> + - prctl(PR_SET_MM, PR_SET_MM_MAP_SIZE, ...) struct prctl_mm_map fields.
>>>> All the PR_SET_MM options that specify pointers (PR_SET_MM_START_CODE,
>>>> PR_SET_MM_END_CODE, ...) should be excluded as well. AFAICT (but don't take my
>>>> word for it), that's all of them except PR_SET_MM_EXE_FILE. Conversely,
>>>> PR_SET_MM_MAP_SIZE should not be excluded (it does not pass a prctl_mm_map
>>>> struct, and the pointer to unsigned int can be tagged).
>>>>
>>> Agreed, I clearly misread the prctl() man page here. Fill fix in v7.
>>> PR_SET_MM_MAP_SIZE _returns_  struct prctl_mm_map, does not take it as a
>>> parameter.
>> OK. About PR_SET_MM_MAP_SIZE, it neither takes nor returns struct prctl_mm_map.
>> It writes the size of prctl_map to the int pointed to by arg3, and does nothing
>> else. Therefore, there's no need to exclude it.
>>
> Agreed, I missed the word size in my reply: s/_returns_  struct
> prctl_mm_map/_returns_  the size of struct prctl_mm_map/
>
>> BTW I've just realised that the man page is wrong about PR_SET_MM_MAP_SIZE, the
>> pointer to int is passed in arg3, not arg4. Anyone knows where to report that?
>>
>> Thanks,
>> Kevin
>>
>>> Vincenzo
>>>
>>>> Kevin
>>>>
>>>>> +
>>>>> +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';
>>>>> +
>>>>> +           ...
>>>>> +   }
>>>>> +
>>>> _______________________________________________
>>>> linux-arm-kernel mailing list
>>>> linux-arm-kernel@lists.infradead.org
>>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v6 1/2] arm64: Define Documentation/arm64/tagged-address-abi.rst
  2019-07-25 13:50   ` [PATCH v6 1/2] arm64: Define Documentation/arm64/tagged-address-abi.rst Vincenzo Frascino
  2019-07-30 10:32     ` Kevin Brodsky
@ 2019-07-31 16:43     ` Dave Hansen
  2019-08-02 10:08       ` Catalin Marinas
  1 sibling, 1 reply; 10+ messages in thread
From: Dave Hansen @ 2019-07-31 16:43 UTC (permalink / raw)
  To: Vincenzo Frascino, linux-arm-kernel, linux-doc, linux-mm,
	linux-arch, linux-kselftest, linux-kernel
  Cc: Catalin Marinas, Will Deacon, Andrey Konovalov, Szabolcs Nagy

On 7/25/19 6:50 AM, Vincenzo Frascino wrote:
> 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().

I don't see a lot of description of why this restriction is necessary.
What's the problem with supporting MAP_SHARED?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v6 1/2] arm64: Define Documentation/arm64/tagged-address-abi.rst
  2019-07-31 16:43     ` Dave Hansen
@ 2019-08-02 10:08       ` Catalin Marinas
  0 siblings, 0 replies; 10+ messages in thread
From: Catalin Marinas @ 2019-08-02 10:08 UTC (permalink / raw)
  To: Dave Hansen
  Cc: Vincenzo Frascino, linux-arm-kernel, linux-doc, linux-mm,
	linux-arch, linux-kselftest, linux-kernel, Will Deacon,
	Andrey Konovalov, Szabolcs Nagy

Hi Dave,

On Wed, Jul 31, 2019 at 09:43:46AM -0700, Dave Hansen wrote:
> On 7/25/19 6:50 AM, Vincenzo Frascino wrote:
> > 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().
> 
> I don't see a lot of description of why this restriction is necessary.
> What's the problem with supporting MAP_SHARED?

We could support MAP_SHARED | MAP_ANONYMOUS (and based on some internal
discussions, this would be fine with the hardware memory tagging as
well). What we don't want in the ABI is to support file mmap() for
top-byte-ignore (or MTE). If you see a use-case, please let us know.

-- 
Catalin

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2019-08-02 10:08 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <cover.1563904656.git.andreyknvl@google.com>
2019-07-25 13:50 ` [PATCH v6 0/2] arm64 relaxed ABI Vincenzo Frascino
2019-07-25 13:50   ` [PATCH v6 1/2] arm64: Define Documentation/arm64/tagged-address-abi.rst Vincenzo Frascino
2019-07-30 10:32     ` Kevin Brodsky
2019-07-30 13:25       ` Vincenzo Frascino
2019-07-30 13:57         ` Kevin Brodsky
2019-07-30 14:24           ` Vincenzo Frascino
2019-07-30 14:48             ` Kevin Brodsky
2019-07-31 16:43     ` Dave Hansen
2019-08-02 10:08       ` Catalin Marinas
2019-07-25 13:50   ` [PATCH v6 2/2] arm64: Relax Documentation/arm64/tagged-pointers.rst Vincenzo Frascino

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox