arm64: untag user pointers passed to the kernel

arch/arm64/kernel/process.c void arch_setup_new_exec(void) · line 572

Quoted 13 lines · Andrey Konovalov

arch/arm64/kernel/process.c +13 -0

+long set_tagged_addr_ctrl(unsigned long arg)
+{
+	if (!tagged_addr_prctl_allowed)
+		return -EINVAL;
+	if (is_compat_task())
+		return -EINVAL;
+	if (arg & ~PR_TAGGED_ADDR_ENABLE)
+		return -EINVAL;
+
+	update_thread_flag(TIF_TAGGED_ADDR, arg & PR_TAGGED_ADDR_ENABLE);
+
+	return 0;
+}

Instead of a plain enable/disable, a more flexible ABI would be to have the tag mask be passed in. That way, an implementation that has a flexible tag size can select it. It also ensures that userspace actually knows what the tag size is and isn't surprised if a hardware implementation changes the tag size or position.

Also, this whole set deals with tagging/untagging, but there's an effective loss of address space when you do this. Is that dealt with anywhere? How do we ensure that allocations don't get placed at a tagged address before this gets turned on? Where's that checking?

This patch series only changes what is allowed or not at the syscall interface. It does not change the address space size. On arm64, TBI (Top Byte Ignore) has always been enabled for userspace, so it has never been possible to use the upper 8 bits of user pointers for addressing.

If other architectures were to support a similar functionality, then I agree that a common and more generic interface (if needed) would be helpful, but as it stands this is an arm64-specific prctl, and on arm64 the address tag is defined by the architecture as bits [63:56].

Quoted 4 lines · Kevin Brodsky

This patch series only changes what is allowed or not at the syscall interface. It does not change the address space size. On arm64, TBI (Top Byte Ignore) has always been enabled for userspace, so it has never been possible to use the upper 8 bits of user pointers for addressing.

Oh, so does the address space that's available already chop that out?

Quoted 4 lines

If other architectures were to support a similar functionality, then I agree that a common and more generic interface (if needed) would be helpful, but as it stands this is an arm64-specific prctl, and on arm64 the address tag is defined by the architecture as bits [63:56].

It should then be an arch_prctl(), no?

From: Catalin Marinas <[email protected]>

It is not desirable to relax the ABI to allow tagged user addresses into the kernel indiscriminately. This patch introduces a prctl() interface for enabling or disabling the tagged ABI with a global sysctl control for preventing applications from enabling the relaxed ABI (meant for testing user-space prctl() return error checking without reconfiguring the kernel). The ABI properties are inherited by threads of the same application and fork()'ed children but cleared on execve(). A Kconfig option allows the overall disabling of the relaxed ABI.

The PR_SET_TAGGED_ADDR_CTRL will be expanded in the future to handle MTE-specific settings like imprecise vs precise exceptions.

Reviewed-by Kees Cook <[email protected]>

Signed-off-by Catalin Marinas <[email protected]>

Signed-off-by Andrey Konovalov <[email protected]>

From 1b3f57ab0c2c51f8b31c19fb34d270e1f3ee57fe Mon Sep 17 00:00:00 2001

This patch is a part of a series that extends kernel ABI to allow to pass tagged user pointers (with the top byte set to something else other than 0x00) as syscall arguments.

This patch allows tagged pointers to be passed to the following memory
syscalls: get_mempolicy, madvise, mbind, mincore, mlock, mlock2, mprotect, mremap, msync, munlock, move_pages.

The mmap and mremap syscalls do not currently accept tagged addresses. Architectures may interpret the tag as a background colour for the corresponding vma.

Reviewed-by Khalid Aziz <[email protected]>

Reviewed-by Vincenzo Frascino <[email protected]>

Reviewed-by Catalin Marinas <[email protected]>

Reviewed-by Kees Cook <[email protected]>

Signed-off-by Andrey Konovalov <[email protected]>

---
 mm/madvise.c   | 2 ++
 mm/mempolicy.c | 3 +++
 mm/migrate.c   | 2 +-
 mm/mincore.c   | 2 ++
 mm/mlock.c     | 4 ++++
 mm/mprotect.c  | 2 ++
 mm/mremap.c    | 7 +++++++
 mm/msync.c     | 2 ++
 8 files changed, 23 insertions(+), 1 deletion(-)

From 9a5286acaa638c6a917d96986bf28dad35e24a0c Mon Sep 17 00:00:00 2001

This patch is a part of a series that extends kernel ABI to allow to pass tagged user pointers (with the top byte set to something else other than 0x00) as syscall arguments.

mlx4_get_umem_mr() uses provided user pointers for vma lookups, which can only by done with untagged pointers.

Untag user pointers in this function.

Reviewed-by Jason Gunthorpe <[email protected]>

Acked-by Catalin Marinas <[email protected]>

Reviewed-by Kees Cook <[email protected]>

Signed-off-by Andrey Konovalov <[email protected]>

---
 drivers/infiniband/hw/mlx4/mr.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Quoted 9 lines · Andrey Konovalov

This patch is a part of a series that extends kernel ABI to allow to pass tagged user pointers (with the top byte set to something else other than 0x00) as syscall arguments.

mlx4_get_umem_mr() uses provided user pointers for vma lookups, which can only by done with untagged pointers.

Untag user pointers in this function.

Quoted 7 lines

Reviewed-by Jason Gunthorpe <[email protected]>

Acked-by Catalin Marinas <[email protected]>

Reviewed-by Kees Cook <[email protected]>

Signed-off-by Andrey Konovalov <[email protected]>

---
 drivers/infiniband/hw/mlx4/mr.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Applied to rdma-for next, please don't sent it via other trees :)

Thanks,
Jason

=== Overview

arm64 has a feature called Top Byte Ignore, which allows to embed pointer tags into the top byte of each pointer. Userspace programs (such as HWASan, a memory debugging tool [1]) might use this feature and pass tagged user pointers to the kernel through syscalls or other interfaces.

Right now the kernel is already able to handle user faults with tagged pointers, due to these patches:

1. 81cddd65 ("arm64: traps: fix userspace cache maintenance emulation on a tagged pointer")
2. 7dcd9dd8 ("arm64: hw_breakpoint: fix watchpoint matching for tagged pointers")
3. 276e9327 ("arm64: entry: improve data abort handling of tagged pointers")

This patchset extends tagged pointer support to syscall arguments.

As per the proposed ABI change [3], tagged pointers are only allowed to be passed to syscalls when they point to memory ranges obtained by anonymous mmap() or sbrk() (see the patchset [3] for more details).

For non-memory syscalls this is done by untaging user pointers when the kernel performs pointer checking to find out whether the pointer comes from userspace (most notably in access_ok). The untagging is done only when the pointer is being checked, the tag is preserved as the pointer makes its way through the kernel and stays tagged when the kernel dereferences the pointer when perfoming user memory accesses.

The mmap and mremap (only new_addr) syscalls do not currently accept tagged addresses. Architectures may interpret the tag as a background colour for the corresponding vma.

Other memory syscalls (mprotect, etc.) don't do user memory accesses but rather deal with memory ranges, and untagged pointers are better suited to describe memory ranges internally. Thus for memory syscalls we untag pointers completely when they enter the kernel.

=== Other approaches

One of the alternative approaches to untagging that was considered is to completely strip the pointer tag as the pointer enters the kernel with some kind of a syscall wrapper, but that won't work with the countless number of different ioctl calls. With this approach we would need a custom wrapper for each ioctl variation, which doesn't seem practical.

An alternative approach to untagging pointers in memory syscalls prologues is to inspead allow tagged pointers to be passed to find_vma() (and other vma related functions) and untag them there. Unfortunately, a lot of find_vma() callers then compare or subtract the returned vma start and end fields against the pointer that was being searched. Thus this approach would still require changing all find_vma() callers.

=== Testing

The following testing approaches has been taken to find potential issues with user pointer untagging:

1. Static testing (with sparse [2] and separately with a custom static analyzer based on Clang) to track casts of __user pointers to integer types to find places where untagging needs to be done.

2. Static testing with grep to find parts of the kernel that call find_vma() (and other similar functions) or directly compare against vm_start/vm_end fields of vma.

3. Static testing with grep to find parts of the kernel that compare user pointers with TASK_SIZE or other similar consts and macros.

4. Dynamic testing: adding BUG_ON(has_tag(addr)) to find_vma() and running a modified syzkaller version that passes tagged pointers to the kernel.

Based on the results of the testing the requried patches have been added to the patchset.

=== Notes

This patchset is meant to be merged together with "arm64 relaxed ABI" [3].

This patchset is a prerequisite for ARM's memory tagging hardware feature support [4].

This patchset has been merged into the Pixel 2 & 3 kernel trees and is now being used to enable testing of Pixel phones with HWASan.

Thanks!

[1] http://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html

[2] https://github.com/lucvoo/sparse-dev/commit/5f960cb10f56ec2017c128ef9d16060e0145f292

[3] https://lkml.org/lkml/2019/6/12/745

[4] https://community.arm.com/processors/b/blog/posts/arm-a-profile-architecture-2018-developments-armv85a

=== History

Changes in v19:
- Rebased onto 7b5cf701 (5.3-rc1+).

Changes in v18:
- Reverted the selftest back to not using the LD_PRELOAD approach.
- Added prctl(PR_SET_TAGGED_ADDR_CTRL) call to the selftest.
- Reworded the patch descriptions to make them less oriented on arm64 only.
- Catalin's patch: "I added a Kconfig option and dropped the prctl args zero check. There is some minor clean-up as well".

Changes in v17:
- The "uaccess: add noop untagged_addr definition" patch is dropped, as it was merged into upstream named as "uaccess: add noop untagged_addr definition".
- Merged "mm, arm64: untag user pointers in do_pages_move" into "mm, arm64: untag user pointers passed to memory syscalls".
- Added "arm64: Introduce prctl() options to control the tagged user addresses ABI" patch from Catalin.
- Add tags_lib.so to tools/testing/selftests/arm64/.gitignore.
- Added a comment clarifying untagged in mremap.
- Moved untagging back into mlx4_get_umem_mr() for the IB patch.

Changes in v16:
- Moved untagging for memory syscalls from arm64 wrappers back to generic code.
- Dropped untagging for the following memory syscalls: brk, mmap, munmap; mremap (only dropped for new_address); mmap_pgoff (not used on arm64); remap_file_pages (deprecated); shmat, shmdt (work on shared memory).
- Changed kselftest to LD_PRELOAD a shared library that overrides malloc to return tagged pointers.
- Rebased onto 5.2-rc3.

Changes in v15:
- Removed unnecessary untagging from radeon_ttm_tt_set_userptr().
- Removed unnecessary untagging from amdgpu_ttm_tt_set_userptr().
- Moved untagging to validate_range() in userfaultfd code.
- Moved untagging to ib_uverbs_(re)reg_mr() from mlx4_get_umem_mr().
- Rebased onto 5.1.

Changes in v14:
- Moved untagging for most memory syscalls to an arm64 specific implementation, instead of doing that in the common code.
- Dropped "net, arm64: untag user pointers in tcp_zerocopy_receive", since the provided user pointers don't come from an anonymous map and thus are not covered by this ABI relaxation.
- Dropped "kernel, arm64: untag user pointers in prctl_set_mm*".
- Moved untagging from __check_mem_type() to tee_shm_register().
- Updated untagging for the amdgpu and radeon drivers to cover the MMU notifier, as suggested by Felix.
- Since this ABI relaxation doesn't actually allow tagged instruction pointers, dropped the following patches:
- Dropped "tracing, arm64: untag user pointers in seq_print_user_ip".
- Dropped "uprobes, arm64: untag user pointers in find_active_uprobe".
- Dropped "bpf, arm64: untag user pointers in stack_map_get_build_id_offset".
- Rebased onto 5.1-rc7 (37624b58).

Changes in v13:
- Simplified untagging in tcp_zerocopy_receive().
- Looked at find_vma() callers in drivers/, which allowed to identify a few other places where untagging is needed.
- Added patch "mm, arm64: untag user pointers in get_vaddr_frames".
- Added patch "drm/amdgpu, arm64: untag user pointers in amdgpu_ttm_tt_get_user_pages".
- Added patch "drm/radeon, arm64: untag user pointers in radeon_ttm_tt_pin_userptr".
- Added patch "IB/mlx4, arm64: untag user pointers in mlx4_get_umem_mr".
- Added patch "media/v4l2-core, arm64: untag user pointers in videobuf_dma_contig_user_get".
- Added patch "tee/optee, arm64: untag user pointers in check_mem_type".
- Added patch "vfio/type1, arm64: untag user pointers".

Changes in v12:
- Changed untagging in tcp_zerocopy_receive() to also untag zc->address.
- Fixed untagging in prctl_set_mm* to only untag pointers for vma lookups and validity checks, but leave them as is for actual user space accesses.
- Updated the link to the v2 of the "arm64 relaxed ABI" patchset [3].
- Dropped the documentation patch, as the "arm64 relaxed ABI" patchset [3] handles that.

Changes in v11:
- Added "uprobes, arm64: untag user pointers in find_active_uprobe" patch.
- Added "bpf, arm64: untag user pointers in stack_map_get_build_id_offset" patch.
- Fixed "tracing, arm64: untag user pointers in seq_print_user_ip" to correctly perform subtration with a tagged addr.
- Moved untagged_addr() from SYSCALL_DEFINE3(mprotect) and SYSCALL_DEFINE4(pkey_mprotect) to do_mprotect_pkey().
- Moved untagged_addr() definition for other arches from include/linux/memory.h to include/linux/mm.h.
- Changed untagging in strn*_user() to perform userspace accesses through tagged pointers.
- Updated the documentation to mention that passing tagged pointers to memory syscalls is allowed.
- Updated the test to use malloc'ed memory instead of stack memory.

Changes in v10:
- Added "mm, arm64: untag user pointers passed to memory syscalls" back.
- New patch "fs, arm64: untag user pointers in fs/userfaultfd.c".
- New patch "net, arm64: untag user pointers in tcp_zerocopy_receive".
- New patch "kernel, arm64: untag user pointers in prctl_set_mm*".
- New patch "tracing, arm64: untag user pointers in seq_print_user_ip".

Changes in v9:
- Rebased onto 4.20-rc6.
- Used u64 instead of __u64 in type casts in the untagged_addr macro for arm64.
- Added braces around (addr) in the untagged_addr macro for other arches.

Changes in v8:
- Rebased onto 65102238 (4.20-rc1).
- Added a note to the cover letter on why syscall wrappers/shims that untag user pointers won't work.
- Added a note to the cover letter that this patchset has been merged into the Pixel 2 kernel tree.
- Documentation fixes, in particular added a list of syscalls that don't support tagged user pointers.

Changes in v7:
- Rebased onto 17b57b18 (4.19-rc6).
- Dropped the "arm64: untag user address in __do_user_fault" patch, since the existing patches already handle user faults properly.
- Dropped the "usb, arm64: untag user addresses in devio" patch, since the passed pointer must come from a vma and therefore be untagged.
- Dropped the "arm64: annotate user pointers casts detected by sparse" patch (see the discussion to the replies of the v6 of this patchset).
- Added more context to the cover letter.
- Updated Documentation/arm64/tagged-pointers.txt.

Changes in v6:
- Added annotations for user pointer casts found by sparse.
- Rebased onto 050cdc6c (4.19-rc1+).

Changes in v5:
- Added 3 new patches that add untagging to places found with static analysis.
- Rebased onto 44c929e1 (4.18-rc8).

Changes in v4:
- Added a selftest for checking that passing tagged pointers to the kernel succeeds.
- Rebased onto 81e97f013 (4.18-rc1+).

Changes in v3:
- Rebased onto e5c51f30 (4.17-rc6+).
- Added linux-arch@ to the list of recipients.

Changes in v2:
- Rebased onto 2d618bdf (4.17-rc3+).
- Removed excessive untagging in gup.c.
- Removed untagging pointers returned from __uaccess_mask_ptr.

Changes in v1:
- Rebased onto 4.17-rc1.

Changes in RFC v2:
- Added "#ifndef untagged_addr..." fallback in linux/uaccess.h instead of defining it for each arch individually.
- Updated Documentation/arm64/tagged-pointers.txt.
- Dropped "mm, arm64: untag user addresses in memory syscalls".
- Rebased onto 3eb2ce82 (4.16-rc7).

Signed-off-by Andrey Konovalov <[email protected]>

Andrey Konovalov (14):
arm64: untag user pointers in access_ok and __uaccess_mask_ptr
lib: untag user pointers in strn*_user
mm: untag user pointers passed to memory syscalls
mm: untag user pointers in mm/gup.c
mm: untag user pointers in get_vaddr_frames
fs/namespace: untag user pointers in copy_mount_options
userfaultfd: untag user pointers
drm/amdgpu: untag user pointers
drm/radeon: untag user pointers in radeon_gem_userptr_ioctl IB/mlx4: untag user pointers in mlx4_get_umem_mr media/v4l2-core: untag user pointers in videobuf_dma_contig_user_get tee/shm: untag user pointers in tee_shm_register vfio/type1: untag user pointers in vaddr_get_pfn selftests, arm64: add a selftest for passing tagged pointers to kernel

Catalin Marinas (1):
arm64: Introduce prctl() options to control the tagged user addresses
ABI

 arch/arm64/Kconfig                            |  9 +++
 arch/arm64/include/asm/processor.h            |  8 ++
 arch/arm64/include/asm/thread_info.h          |  1 +
 arch/arm64/include/asm/uaccess.h              | 12 ++-
 arch/arm64/kernel/process.c                   | 73 +++++++++++++++++++
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c       |  2 +
 drivers/gpu/drm/radeon/radeon_gem.c           |  2 +
 drivers/infiniband/hw/mlx4/mr.c               |  7 +-
 drivers/media/v4l2-core/videobuf-dma-contig.c |  9 ++-
 drivers/tee/tee_shm.c                         |  1 +
 drivers/vfio/vfio_iommu_type1.c               |  2 +
 fs/namespace.c                                |  2 +-
 fs/userfaultfd.c                              | 22 +++---
 include/uapi/linux/prctl.h                    |  5 ++
 kernel/sys.c                                  | 12 +++
 lib/strncpy_from_user.c                       |  3 +-
 lib/strnlen_user.c                            |  3 +-
 mm/frame_vector.c                             |  2 +
 mm/gup.c                                      |  4 +
 mm/madvise.c                                  |  2 +
 mm/mempolicy.c                                |  3 +
 mm/migrate.c                                  |  2 +-
 mm/mincore.c                                  |  2 +
 mm/mlock.c                                    |  4 +
 mm/mprotect.c                                 |  2 +
 mm/mremap.c                                   |  7 ++
 mm/msync.c                                    |  2 +
 tools/testing/selftests/arm64/.gitignore      |  1 +
 tools/testing/selftests/arm64/Makefile        | 11 +++
 .../testing/selftests/arm64/run_tags_test.sh  | 12 +++
 tools/testing/selftests/arm64/tags_test.c     | 29 ++++++++
 32 files changed, 233 insertions(+), 25 deletions(-)
 create mode 100644 tools/testing/selftests/arm64/.gitignore
 create mode 100644 tools/testing/selftests/arm64/Makefile
 create mode 100755 tools/testing/selftests/arm64/run_tags_test.sh
 create mode 100644 tools/testing/selftests/arm64/tags_test.c

--
2.22.0.709.g102302147b-goog

Quoted 19 lines · Andrey Konovalov

=== Overview

arm64 has a feature called Top Byte Ignore, which allows to embed pointer tags into the top byte of each pointer. Userspace programs (such as HWASan, a memory debugging tool [1]) might use this feature and pass tagged user pointers to the kernel through syscalls or other interfaces.

Right now the kernel is already able to handle user faults with tagged pointers, due to these patches:

1. 81cddd65 ("arm64: traps: fix userspace cache maintenance emulation on a tagged pointer")
2. 7dcd9dd8 ("arm64: hw_breakpoint: fix watchpoint matching for tagged pointers")
3. 276e9327 ("arm64: entry: improve data abort handling of tagged pointers")

This patchset extends tagged pointer support to syscall arguments.

Do you think this is ready to be merged?

Should this go through the mm or the arm tree?

Hi Andrey,

Quoted 20 lines · Andrey Konovalov

Quoted 19 lines · Andrey Konovalov

=== Overview

arm64 has a feature called Top Byte Ignore, which allows to embed pointer tags into the top byte of each pointer. Userspace programs (such as HWASan, a memory debugging tool [1]) might use this feature and pass tagged user pointers to the kernel through syscalls or other interfaces.

Right now the kernel is already able to handle user faults with tagged pointers, due to these patches:

1. 81cddd65 ("arm64: traps: fix userspace cache maintenance emulation on a tagged pointer")
2. 7dcd9dd8 ("arm64: hw_breakpoint: fix watchpoint matching for tagged pointers")
3. 276e9327 ("arm64: entry: improve data abort handling of tagged pointers")

This patchset extends tagged pointer support to syscall arguments.

[...]

Quoted 3 lines

Do you think this is ready to be merged?

Should this go through the mm or the arm tree?

I would certainly prefer to take at least the arm64 bits via the arm64 tree (i.e. patches 1, 2 and 15). We also need a Documentation patch describing the new ABI.

Will

Quoted 26 lines · Will Deacon

Quoted 20 lines · Andrey Konovalov

Quoted 19 lines · Andrey Konovalov

=== Overview

arm64 has a feature called Top Byte Ignore, which allows to embed pointer tags into the top byte of each pointer. Userspace programs (such as HWASan, a memory debugging tool [1]) might use this feature and pass tagged user pointers to the kernel through syscalls or other interfaces.

Right now the kernel is already able to handle user faults with tagged pointers, due to these patches:

1. 81cddd65 ("arm64: traps: fix userspace cache maintenance emulation on a tagged pointer")
2. 7dcd9dd8 ("arm64: hw_breakpoint: fix watchpoint matching for tagged pointers")
3. 276e9327 ("arm64: entry: improve data abort handling of tagged pointers")

This patchset extends tagged pointer support to syscall arguments.

[...]

Quoted 2 lines

Do you think this is ready to be merged?

Quoted 5 lines

Quoted 1 line

Should this go through the mm or the arm tree?

I would certainly prefer to take at least the arm64 bits via the arm64 tree (i.e. patches 1, 2 and 15). We also need a Documentation patch describing the new ABI.

Sounds good! Should I post those patches together with the Documentation patches from Vincenzo as a separate patchset?

Quoted 33 lines · Andrey Konovalov

Quoted 31 lines · Will Deacon

Quoted 20 lines · Andrey Konovalov

Quoted 19 lines · Andrey Konovalov

=== Overview

arm64 has a feature called Top Byte Ignore, which allows to embed pointer tags into the top byte of each pointer. Userspace programs (such as HWASan, a memory debugging tool [1]) might use this feature and pass tagged user pointers to the kernel through syscalls or other interfaces.

Right now the kernel is already able to handle user faults with tagged pointers, due to these patches:

1. 81cddd65 ("arm64: traps: fix userspace cache maintenance emulation on a tagged pointer")
2. 7dcd9dd8 ("arm64: hw_breakpoint: fix watchpoint matching for tagged pointers")
3. 276e9327 ("arm64: entry: improve data abort handling of tagged pointers")

This patchset extends tagged pointer support to syscall arguments.

[...]

Quoted 3 lines

Do you think this is ready to be merged?

Should this go through the mm or the arm tree?

I would certainly prefer to take at least the arm64 bits via the arm64 tree (i.e. patches 1, 2 and 15). We also need a Documentation patch describing the new ABI.

Quoted 2 lines

Sounds good! Should I post those patches together with the Documentation patches from Vincenzo as a separate patchset?

Yes, please (although as you say below, we need a new version of those patches from Vincenzo to address the feedback on v5). The other thing I should say is that I'd be happy to queue the other patches in the series too, but some of them are missing acks from the relevant maintainers (e.g. the mm/ and fs/ changes).

Will

Quoted 8 lines · Andrey Konovalov

Quoted 6 lines · Will Deacon

Quoted 1 line · Andrey Konovalov

Should this go through the mm or the arm tree?

I would certainly prefer to take at least the arm64 bits via the arm64 tree (i.e. patches 1, 2 and 15). We also need a Documentation patch describing the new ABI.

Quoted 2 lines

Sounds good! Should I post those patches together with the Documentation patches from Vincenzo as a separate patchset?

Yes, please (although as you say below, we need a new version of those patches from Vincenzo to address the feedback on v5). The other thing I should say is that I'd be happy to queue the other patches in the series too, but some of them are missing acks from the relevant maintainers (e.g. the mm/ and fs/ changes).

Quoted 9 lines · Will Deacon

Quoted 8 lines · Andrey Konovalov

Quoted 6 lines · Will Deacon

Quoted 1 line · Andrey Konovalov

Should this go through the mm or the arm tree?

I would certainly prefer to take at least the arm64 bits via the arm64 tree (i.e. patches 1, 2 and 15). We also need a Documentation patch describing the new ABI.

Quoted 8 lines

Quoted 2 lines

Sounds good! Should I post those patches together with the Documentation patches from Vincenzo as a separate patchset?

Yes, please (although as you say below, we need a new version of those patches from Vincenzo to address the feedback on v5). The other thing I should say is that I'd be happy to queue the other patches in the series too, but some of them are missing acks from the relevant maintainers (e.g. the mm/ and fs/ changes).

Ok, I've queued patches 1, 2, and 15 on a stable branch here:

https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/log/?h=for-next/tbi

which should find its way into -next shortly via our for-next/core branch. If you want to make changes, please send additional patches on top.

This is targetting 5.4, but I will drop it before the merge window if we don't have both of the following in place:

* Updated ABI documentation with Acks from Catalin and Kevin

* The other patches in the series either Acked (so I can pick them up) or queued via some other tree(s) for 5.4.

Make sense?

Cheers,

Will

Quoted 24 lines · Will Deacon

Quoted 9 lines · Will Deacon

Quoted 8 lines · Andrey Konovalov

Quoted 6 lines · Will Deacon

Quoted 1 line · Andrey Konovalov

Should this go through the mm or the arm tree?

I would certainly prefer to take at least the arm64 bits via the arm64 tree (i.e. patches 1, 2 and 15). We also need a Documentation patch describing the new ABI.

Quoted 8 lines

Quoted 2 lines

Sounds good! Should I post those patches together with the Documentation patches from Vincenzo as a separate patchset?

Yes, please (although as you say below, we need a new version of those patches from Vincenzo to address the feedback on v5). The other thing I should say is that I'd be happy to queue the other patches in the series too, but some of them are missing acks from the relevant maintainers (e.g. the mm/ and fs/ changes).

Ok, I've queued patches 1, 2, and 15 on a stable branch here:

https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/log/?h=for-next/tbi

Quoted 7 lines

which should find its way into -next shortly via our for-next/core branch. If you want to make changes, please send additional patches on top.

This is targetting 5.4, but I will drop it before the merge window if we don't have both of the following in place:

* Updated ABI documentation with Acks from Catalin and Kevin

Catalin has posted a new version today.

Quoted 2 lines

* The other patches in the series either Acked (so I can pick them up) or queued via some other tree(s) for 5.4.

So we have the following patches in this series:

1. arm64: untag user pointers in access_ok and __uaccess_mask_ptr
2. arm64: Introduce prctl() options to control the tagged user addresses ABI
3. lib: untag user pointers in strn*_user
4. mm: untag user pointers passed to memory syscalls
5. mm: untag user pointers in mm/gup.c
6. mm: untag user pointers in get_vaddr_frames
7. fs/namespace: untag user pointers in copy_mount_options
8. userfaultfd: untag user pointers
9. drm/amdgpu: untag user pointers
10. drm/radeon: untag user pointers in radeon_gem_userptr_ioctl
11. IB/mlx4: untag user pointers in mlx4_get_umem_mr
12. media/v4l2-core: untag user pointers in videobuf_dma_contig_user_get
13. tee/shm: untag user pointers in tee_shm_register
14. vfio/type1: untag user pointers in vaddr_get_pfn
15. selftests, arm64: add a selftest for passing tagged pointers to kernel

1, 2 and 15 have been picked by Will.

11 has been picked up by Jason.

9, 10, 12, 13 and 14 have acks from their subsystem maintainers.

3 touches generic lib code, I'm not sure if there's a dedicated maintainer for that.

The ones that are left are the mm ones: 4, 5, 6, 7 and 8.

Andrew, could you take a look and give your Acked-by or pick them up directly?

Quoted 3 lines

The ones that are left are the mm ones: 4, 5, 6, 7 and 8.

Andrew, could you take a look and give your Acked-by or pick them up directly?

Given the subsystem Acks, it seems like 3-10 and 12 could all just go via Andrew? I hope he agrees. :)

Quoted 6 lines · On Thu, 8 Aug 2019 14:12:19 -0700 Kees Cook <[email protected]> wrote:

Quoted 3 lines

The ones that are left are the mm ones: 4, 5, 6, 7 and 8.

Andrew, could you take a look and give your Acked-by or pick them up directly?

Given the subsystem Acks, it seems like 3-10 and 12 could all just go via Andrew? I hope he agrees. :)

I'll grab everything that has not yet appeared in linux-next. If more of these patches appear in linux-next I'll drop those as well.

The review discussion against " [PATCH v19 02/15] arm64: Introduce prctl() options to control the tagged user addresses ABI" has petered out inconclusively. prctl() vs arch_prctl().

Quoted 12 lines · Andrew Morton

Quoted 6 lines · On Thu, 8 Aug 2019 14:12:19 -0700 Kees Cook <[email protected]> wrote:

Quoted 3 lines

The ones that are left are the mm ones: 4, 5, 6, 7 and 8.

Andrew, could you take a look and give your Acked-by or pick them up directly?

Given the subsystem Acks, it seems like 3-10 and 12 could all just go via Andrew? I hope he agrees. :)

I'll grab everything that has not yet appeared in linux-next. If more of these patches appear in linux-next I'll drop those as well.

Quoted 3 lines

The review discussion against " [PATCH v19 02/15] arm64: Introduce prctl() options to control the tagged user addresses ABI" has petered out inconclusively. prctl() vs arch_prctl().

I've always disliked arch_prctl() existing at all. Given that tagging is likely to be a multi-architectural feature, it seems like the controls should live in prctl() to me.

Quoted 13 lines · Kees Cook

Quoted 12 lines · Andrew Morton

Quoted 6 lines · On Thu, 8 Aug 2019 14:12:19 -0700 Kees Cook <[email protected]> wrote:

Quoted 3 lines

The ones that are left are the mm ones: 4, 5, 6, 7 and 8.

Andrew, could you take a look and give your Acked-by or pick them up directly?

Given the subsystem Acks, it seems like 3-10 and 12 could all just go via Andrew? I hope he agrees. :)

I'll grab everything that has not yet appeared in linux-next. If more of these patches appear in linux-next I'll drop those as well.

Quoted 7 lines

Quoted 3 lines

The review discussion against " [PATCH v19 02/15] arm64: Introduce prctl() options to control the tagged user addresses ABI" has petered out inconclusively. prctl() vs arch_prctl().

I've always disliked arch_prctl() existing at all. Given that tagging is likely to be a multi-architectural feature, it seems like the controls should live in prctl() to me.

It took a bit of grep'ing to figure out what Dave H meant by arch_prctl(). It's an x86-specific syscall which we do not have on arm64 (and possibly any other architecture). Actually, we don't have any arm64 specific syscalls, only the generic unistd.h, hence the confusion. For other arm64-specific prctls like SVE we used the generic sys_prctl() and I can see x86 not being consistent either (PR_MPX_ENABLE_MANAGEMENT).

In general I disagree with adding any arm64-specific syscalls but in this instance it can't even be justified. I'd rather see some clean-up similar to arch_ptrace/ptrace_request than introducing new syscall numbers (but as I suggested in my reply to Dave, that's for another patch series).

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 <[email protected]>

Cc Will Deacon <[email protected]>

CC Andrey Konovalov <[email protected]>

Signed-off-by Vincenzo Frascino <[email protected]>

Acked-by Szabolcs Nagy <[email protected]>

---
  Documentation/arm64/tagged-address-abi.rst | 148 +++++++++++++++++++++
  1 file changed, 148 insertions(+)

create mode 100644 Documentation/arm64/tagged-address-abi.rst

Documentation/arm64/tagged-address-abi.rst new file +11 -0

new file mode 100644
index 000000000000..a8ecb991de82
@@ -0,0 +1,148 @@
+========================
+ARM64 TAGGED ADDRESS ABI
+========================
+
+Author: Vincenzo Frascino <[email protected]>
+
+Date: 25 July 2019
+
+This document describes the usage and semantics of the Tagged Address
+ABI on arm64.
+

Documentation/arm64/tagged-address-abi.rst new file +36 -0

+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.

Documentation/arm64/tagged-address-abi.rst new file +6 -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.

Documentation/arm64/tagged-address-abi.rst new file +11 -0

+ - 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

Documentation/arm64/tagged-address-abi.rst new file +32 -0

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

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

Quoted 47 lines · Vincenzo Frascino

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 <[email protected]>

Cc Will Deacon <[email protected]>

CC Andrey Konovalov <[email protected]>

Signed-off-by Vincenzo Frascino <[email protected]>

Acked-by Szabolcs Nagy <[email protected]>

---
  Documentation/arm64/tagged-address-abi.rst | 148 +++++++++++++++++++++

  1 file changed, 148 insertions(+)

  create mode 100644 Documentation/arm64/tagged-address-abi.rst

Documentation/arm64/tagged-address-abi.rst new file +15 -0

new file mode 100644
index 000000000000..a8ecb991de82
@@ -0,0 +1,148 @@
+========================
+ARM64 TAGGED ADDRESS ABI
+========================
+
+Author: Vincenzo Frascino <[email protected]>
+
+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

Documentation/arm64/tagged-address-abi.rst line 21

Quoted 6 lines

Documentation/arm64/tagged-address-abi.rst new file +6 -0

+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).

Quoted 42 lines

Documentation/arm64/tagged-address-abi.rst new file +42 -0

+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.

Documentation/arm64/tagged-address-abi.rst line 67

Quoted 4 lines

Documentation/arm64/tagged-address-abi.rst new file +4 -0

+
+   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.

Documentation/arm64/tagged-address-abi.rst line 71

Quoted 4 lines

Documentation/arm64/tagged-address-abi.rst new file +4 -0

+- **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).

Documentation/arm64/tagged-address-abi.rst line 79

Quoted 8 lines

Documentation/arm64/tagged-address-abi.rst new file +8 -0

+
+    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.

Quoted 12 lines

Documentation/arm64/tagged-address-abi.rst new file +8 -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

Documentation/arm64/tagged-address-abi.rst new file +3 -0

+of the same application and fork()'ed children but cleared by execve().
+
+When a process has successfully opted into the new ABI by invoking

Quoted 6 lines

Documentation/arm64/tagged-address-abi.rst new file +3 -0

+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?

Quoted 19 lines

+ - 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.

Documentation/arm64/tagged-address-abi.rst new file +13 -0

+
+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.

Quoted 2 lines

+ - 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

Quoted 22 lines

Documentation/arm64/tagged-address-abi.rst new file +22 -0

+
+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;
+

Documentation/arm64/tagged-address-abi.rst line 148

Quoted 10 lines

Documentation/arm64/tagged-address-abi.rst new file +10 -0

+           if (tbi_enabled)
+                   tag = rand() & 0xff;
+
+           ptr = (char *)((unsigned long)ptr | (tag << TAG_SHIFT));
+
+           *ptr = 'a';
+
+           ...
+   }
+

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Hi Kevin,

Quoted 50 lines · Kevin Brodsky

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

Quoted 46 lines · Vincenzo Frascino

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 <[email protected]>

Cc Will Deacon <[email protected]>

CC Andrey Konovalov <[email protected]>

Signed-off-by Vincenzo Frascino <[email protected]>

Acked-by Szabolcs Nagy <[email protected]>

---
   Documentation/arm64/tagged-address-abi.rst | 148 +++++++++++++++++++++
   1 file changed, 148 insertions(+)

  create mode 100644 Documentation/arm64/tagged-address-abi.rst

Documentation/arm64/tagged-address-abi.rst new file +14 -0

new file mode 100644
index 000000000000..a8ecb991de82
@@ -0,0 +1,148 @@
+========================
+ARM64 TAGGED ADDRESS ABI
+========================
+
+Author: Vincenzo Frascino <[email protected]>
+
+Date: 25 July 2019
+
+This document describes the usage and semantics of the Tagged Address
+ABI on arm64.
+
+1. Introduction
+---------------
+

Quoted 9 lines

Documentation/arm64/tagged-address-abi.rst line 21

Quoted 7 lines

Documentation/arm64/tagged-address-abi.rst new file +7 -0

+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.

Quoted 41 lines

Quoted 41 lines

Documentation/arm64/tagged-address-abi.rst new file +41 -0

+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

Quoted 9 lines

Documentation/arm64/tagged-address-abi.rst line 67

Quoted 5 lines

Documentation/arm64/tagged-address-abi.rst new file +5 -0

+    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 ;)

Quoted 10 lines

Documentation/arm64/tagged-address-abi.rst line 71

Quoted 5 lines

Documentation/arm64/tagged-address-abi.rst new file +4 -0

+- **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().

Quoted 12 lines

Documentation/arm64/tagged-address-abi.rst line 79

Quoted 9 lines

Documentation/arm64/tagged-address-abi.rst new file +8 -0

+
+    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.

Quoted 11 lines

Quoted 11 lines

Documentation/arm64/tagged-address-abi.rst new file +8 -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

Documentation/arm64/tagged-address-abi.rst new file +2 -0

+of the same application and fork()'ed children but cleared by execve().
+

Quoted 9 lines

Quoted 7 lines

Documentation/arm64/tagged-address-abi.rst new file +4 -0

+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.

Quoted 19 lines

Quoted 19 lines

+ - 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.

Documentation/arm64/tagged-address-abi.rst new file +12 -0

+
+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.

Quoted 9 lines

Quoted 3 lines

+ - 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

Quoted 38 lines

Kevin

Quoted 22 lines

Documentation/arm64/tagged-address-abi.rst new file +22 -0

+
+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;
+

Documentation/arm64/tagged-address-abi.rst line 148

Quoted 10 lines

Documentation/arm64/tagged-address-abi.rst new file +10 -0

+           if (tbi_enabled)
+                   tag = rand() & 0xff;
+
+           ptr = (char *)((unsigned long)ptr | (tag << TAG_SHIFT));
+
+           *ptr = 'a';
+
+           ...
+   }
+

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Quoted 114 lines · Vincenzo Frascino

Hi Kevin,

Quoted 50 lines · Kevin Brodsky

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

Quoted 46 lines · Vincenzo Frascino

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 <[email protected]>

Cc Will Deacon <[email protected]>

CC Andrey Konovalov <[email protected]>

Signed-off-by Vincenzo Frascino <[email protected]>

Acked-by Szabolcs Nagy <[email protected]>

---
   Documentation/arm64/tagged-address-abi.rst | 148 +++++++++++++++++++++

   1 file changed, 148 insertions(+)

   create mode 100644 Documentation/arm64/tagged-address-abi.rst

Documentation/arm64/tagged-address-abi.rst new file +14 -0

new file mode 100644
index 000000000000..a8ecb991de82
@@ -0,0 +1,148 @@
+========================
+ARM64 TAGGED ADDRESS ABI
+========================
+
+Author: Vincenzo Frascino <[email protected]>
+
+Date: 25 July 2019
+
+This document describes the usage and semantics of the Tagged Address
+ABI on arm64.
+
+1. Introduction
+---------------
+

Quoted 9 lines

Documentation/arm64/tagged-address-abi.rst line 21

Quoted 7 lines

Documentation/arm64/tagged-address-abi.rst new file +7 -0

+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.

Quoted 50 lines

Quoted 36 lines

Documentation/arm64/tagged-address-abi.rst new file +36 -0

+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.

Documentation/arm64/tagged-address-abi.rst line 67

Quoted 10 lines

Documentation/arm64/tagged-address-abi.rst new file +10 -0

+  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.

Quoted 2 lines

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.

Quoted 11 lines

Quoted 11 lines

Quoted 6 lines

Documentation/arm64/tagged-address-abi.rst new file +3 -0

+- **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).

Quoted 4 lines

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).

Quoted 36 lines

Quoted 12 lines

Documentation/arm64/tagged-address-abi.rst line 79

Quoted 9 lines

Documentation/arm64/tagged-address-abi.rst new file +8 -0

+
+    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.

Quoted 11 lines

Quoted 11 lines

Documentation/arm64/tagged-address-abi.rst new file +8 -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

Documentation/arm64/tagged-address-abi.rst new file +2 -0

+of the same application and fork()'ed children but cleared by execve().
+

Quoted 9 lines

Quoted 7 lines

Documentation/arm64/tagged-address-abi.rst new file +4 -0

+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?

Quoted 3 lines

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.

Quoted 28 lines

Quoted 28 lines

Quoted 12 lines

+ - 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.

Documentation/arm64/tagged-address-abi.rst new file +5 -0

+
+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

Documentation/arm64/tagged-address-abi.rst line 116

Quoted 10 lines

Documentation/arm64/tagged-address-abi.rst new file +10 -0

+--------------------------------------
+
+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).

Quoted 3 lines

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

Quoted 40 lines

Vincenzo

Quoted 38 lines

Kevin

Quoted 22 lines

Documentation/arm64/tagged-address-abi.rst new file +22 -0

+
+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;
+

Documentation/arm64/tagged-address-abi.rst line 148

Quoted 10 lines

Documentation/arm64/tagged-address-abi.rst new file +10 -0

+           if (tbi_enabled)
+                   tag = rand() & 0xff;
+
+           ptr = (char *)((unsigned long)ptr | (tag << TAG_SHIFT));
+
+           *ptr = 'a';
+
+           ...
+   }
+

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Hi Kevin,

Quoted 115 lines · Kevin Brodsky

Quoted 114 lines · Vincenzo Frascino

Hi Kevin,

Quoted 50 lines · Kevin Brodsky

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

Quoted 46 lines · Vincenzo Frascino

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 <[email protected]>

Cc Will Deacon <[email protected]>

CC Andrey Konovalov <[email protected]>

Signed-off-by Vincenzo Frascino <[email protected]>

Acked-by Szabolcs Nagy <[email protected]>

---
    Documentation/arm64/tagged-address-abi.rst | 148 +++++++++++++++++++++
    1 file changed, 148 insertions(+)

   create mode 100644 Documentation/arm64/tagged-address-abi.rst

Documentation/arm64/tagged-address-abi.rst new file +14 -0

new file mode 100644
index 000000000000..a8ecb991de82
@@ -0,0 +1,148 @@
+========================
+ARM64 TAGGED ADDRESS ABI
+========================
+
+Author: Vincenzo Frascino <[email protected]>
+
+Date: 25 July 2019
+
+This document describes the usage and semantics of the Tagged Address
+ABI on arm64.
+
+1. Introduction
+---------------
+

Quoted 9 lines

Documentation/arm64/tagged-address-abi.rst line 21

Quoted 7 lines

Documentation/arm64/tagged-address-abi.rst new file +7 -0

+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.

Quoted 50 lines

Quoted 36 lines

Documentation/arm64/tagged-address-abi.rst new file +36 -0

+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.

Documentation/arm64/tagged-address-abi.rst line 67

Quoted 10 lines

Documentation/arm64/tagged-address-abi.rst new file +10 -0

+  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.

Quoted 8 lines

Quoted 2 lines

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.

Quoted 10 lines

Quoted 10 lines

Quoted 10 lines

Quoted 5 lines

Documentation/arm64/tagged-address-abi.rst new file +3 -0

+- **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).

Quoted 9 lines

Quoted 4 lines

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?

Quoted 67 lines

Quoted 35 lines

Quoted 11 lines

Documentation/arm64/tagged-address-abi.rst line 79

Quoted 8 lines

Documentation/arm64/tagged-address-abi.rst new file +8 -0

+
+    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.

Quoted 11 lines

Quoted 11 lines

Documentation/arm64/tagged-address-abi.rst new file +8 -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

Documentation/arm64/tagged-address-abi.rst new file +2 -0

+of the same application and fork()'ed children but cleared by execve().
+

Quoted 9 lines

Quoted 7 lines

Documentation/arm64/tagged-address-abi.rst new file +4 -0

+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?

Quoted 3 lines

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.

Quoted 27 lines

Quoted 27 lines

Quoted 11 lines

+ - 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.

Documentation/arm64/tagged-address-abi.rst new file +5 -0

+
+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

Documentation/arm64/tagged-address-abi.rst line 116

Quoted 10 lines

Documentation/arm64/tagged-address-abi.rst new file +10 -0

+--------------------------------------
+
+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).

Quoted 7 lines

Quoted 3 lines

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/

Quoted 46 lines

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

Quoted 40 lines

Vincenzo

Quoted 38 lines

Kevin

Quoted 22 lines

Documentation/arm64/tagged-address-abi.rst new file +22 -0

+
+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;
+

Documentation/arm64/tagged-address-abi.rst line 148

Quoted 10 lines

Documentation/arm64/tagged-address-abi.rst new file +10 -0

+           if (tbi_enabled)
+                   tag = rand() & 0xff;
+
+           ptr = (char *)((unsigned long)ptr | (tag << TAG_SHIFT));
+
+           *ptr = 'a';
+
+           ...
+   }
+

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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().

Quoted 3 lines · Vincenzo Frascino

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?

The mmap and mremap (only new_addr) syscalls do not currently accept tagged addresses. Architectures may interpret the tag as a background colour for the corresponding vma.

Quoted 3 lines · Andrey Konovalov

The mmap and mremap (only new_addr) syscalls do not currently accept tagged addresses. Architectures may interpret the tag as a background colour for the corresponding vma.

What the heck is a "background colour"? :)

Quoted 4 lines · Dave Hansen

Quoted 3 lines · Andrey Konovalov

The mmap and mremap (only new_addr) syscalls do not currently accept tagged addresses. Architectures may interpret the tag as a background colour for the corresponding vma.

Quoted 1 line

What the heck is a "background colour"? :)

Good point, this is some jargon that we started using for MTE, the idea being that the kernel could set a tag value (specified during mmap()) as "background colour" for anonymous pages allocated in that range.

Anyway, this patch series is not about MTE. Andrey, for v20 (if any), I think it's best to drop this last sentence to avoid any confusion.

Kevin

Quoted 5 lines · Kevin Brodsky

Quoted 4 lines · Dave Hansen

Quoted 3 lines · Andrey Konovalov

The mmap and mremap (only new_addr) syscalls do not currently accept tagged addresses. Architectures may interpret the tag as a background colour for the corresponding vma.

Quoted 7 lines

Quoted 1 line

What the heck is a "background colour"? :)

Good point, this is some jargon that we started using for MTE, the idea being that the kernel could set a tag value (specified during mmap()) as "background colour" for anonymous pages allocated in that range.

Anyway, this patch series is not about MTE. Andrey, for v20 (if any), I think it's best to drop this last sentence to avoid any confusion.

Sure, thanks!

Quoted 7 lines · Andrey Konovalov

Quoted 6 lines · Kevin Brodsky

Quoted 5 lines · Dave Hansen

Quoted 3 lines · Andrey Konovalov

The mmap and mremap (only new_addr) syscalls do not currently accept tagged addresses. Architectures may interpret the tag as a background colour for the corresponding vma.

Quoted 8 lines

Quoted 8 lines

Quoted 1 line

What the heck is a "background colour"? :)

Good point, this is some jargon that we started using for MTE, the idea being that the kernel could set a tag value (specified during mmap()) as "background colour" for anonymous pages allocated in that range.

Anyway, this patch series is not about MTE. Andrey, for v20 (if any), I think it's best to drop this last sentence to avoid any confusion.

OK, but what does that mean for tagged addresses getting passed to mmap/mremap? That sentence read to me like "architectures might allow tags for ...something...". So do we accept tagged addresses into those syscalls?