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 <vincenzo.frascino@xxxxxxx>
> >
> > 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 <will.deacon@xxxxxxx>
> > Cc: Andrey Konovalov <andreyknvl@xxxxxxxxxx>
> > Cc: Szabolcs Nagy <szabolcs.nagy@xxxxxxx>
> > Cc: Kevin Brodsky <kevin.brodsky@xxxxxxx>
> > Signed-off-by: Vincenzo Frascino <vincenzo.frascino@xxxxxxx>
> > [catalin.marinas@xxxxxxx: some rewording, dropped MAP_PRIVATE]
> > Signed-off-by: Catalin Marinas <catalin.marinas@xxxxxxx>
> > ---
> > 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 <vincenzo.frascino@xxxxxxx>
> > +
> > +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
> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
