Hi Dave,

On Wed, May 27, 2020 at 10:17:38PM +0100, Dave P Martin wrote:

Nitpick: maybe say "userspace addresses" again inside the brackets since
kernel addresses have all top bits 1.

+.IP
+The level of support is selected by
+.IR "(unsigned int) arg2" ,

We use (unsigned long) for arg2.

+which can be one of the following:
+.RS
+.TP
+.B 0
+Addresses that are passed
+for the purpose of being dereferenced by the kernel
+must be untagged.
+.TP
+.B PR_TAGGED_ADDR_ENABLE
+Addresses that are passed
+for the purpose of being dereferenced by the kernel
+may be tagged, with the exceptions summarized below.
+.RE
+.IP
+The remaining arguments
+.IR arg3 ", " arg4 " and " arg5
+must all be zero.

Indeed. The above commit didn't have this, we added it later in commit
3e91ec89f527b9870fe42dcbdb74fd389d123a95.

+.IP
+On success, the mode specified in
+.I arg2
+is set for the calling thread and the the return value is 0.
+If the arguments are invalid,
+the mode specified in
+.I arg2
+is unrecognized,
+or if this feature is disabled or unsupported by the kernel,
+the call fails with
+.BR EINVAL .
+.IP
+In particular, if
+.BR prctl ( PR_SET_TAGGED_ADDR_CTRL ,
+0, 0, 0, 0)
+fails with
+.B EINVAL
+then all addresses passed to the kernel must be untagged.
+.IP
+Irrespective of which mode is set,
+addresses passed to certain interfaces
+must always be untagged:

Maybe you could add some extra info from the kernel comment (commit
b2a84de2a2deb76a6a51609845341f508c518c03) along the lines of "... to
avoid the creation of aliasing mappings in userspace).

+.RS
+.IP \(em
+.BR brk (2),
+.BR mmap (2),
+.BR shmat (2),
+and the
+.I new_address
+argument of
+.BR mremap (2).
+.IP
+(Prior to Linux 5.6 these accepted tagged addresses,
+but the behaviour may not be what you expect.
+Don't rely on it.)

shmat() was not part of the subsequent fix
(dcde237319e626d1ec3c9d8b7613032f0fd4663a), it always rejected tagged
address. But I guess it doesn't matter much, the user should not pass
tagged addresses to these syscalls anyway.

You could move shmat() down together with shmdt().

+.IP \(em
+\(oqpolymorphic\(cq interfaces
+that accept pointers to arbitrary types cast to a
+.I void *
+or other generic type, specifically
+.BR prctl (2),
+.BR ioctl (2),
+and in general
+.BR setsockopt (2)
+(only certain specific
+.BR setsockopt (2)
+options allow tagged addresses).
+.IP \(em
+.BR shmdt (2).
+.RE
+.IP
+This list of exclusions may shrink
+when moving from one kernel version to a later kernel version.
+While the kernel may make some guarantees
+for backwards compatibility reasons,
+for the purposes of new software
+the effect of passing tagged addresses to these interfaces
+is unspecified.
+.IP
+The mode set by this call is inherited across
+.BR fork (2)
+and
+.BR clone (2).
+The mode is reset by
+.BR execve (2)
+to 0
+(i.e., tagged addresses not permitted in the user/kernel ABI).
+.IP
+.B Warning:
+Because the compiler or run-time environment
+may make use of address tagging,
+a successful
+.B PR_SET_TAGGED_ADDR_CTRL
+may crash the calling process.

I don't think PR_SET_TAGGED_ADDR_CTRL could crash the calling process.
Rather disabling tagged addresses would break it. If a process is using
tagged addresses but does not pass them to the kernel, it will continue
to do so even when the syscalls accept such addresses.

+The conditions for using it safely are complex and system-dependent.
+Don't use it unless you know what you are doing.

This syscall is intended for the C library if the heap allocator
generates tagged addresses. So it's not a general purpose prctl() random
application code could call. Anyway I'm fine with your warning of not
doing it but you may want to clarify the intent.

+.IP
+For more information, see the kernel source file
+.IR Documentation/arm64/tagged\-address\-abi.rst .
+.\" prctl PR_GET_TAGGED_ADDR_CTRL
+.\" commit 63f0c60379650d82250f22e4cf4137ef3dc4f43d
+.TP
+.BR PR_GET_TAGGED_ADDR_CTRL " (since Linux 5.4, only on arm64)"
+Returns the current tagged address mode
+for the calling thread.
+.IP
+Arguments
+.IR arg2 ", " arg3 ", " arg4 " and " arg5
+must all be zero.
+.IP
+If the arguments are invalid
+or this feature is disabled or unsupported by the kernel,
+the call fails with
+.BR EINVAL .
+In particular, if
+.BR prctl ( PR_GET_TAGGED_ADDR_CTRL ,
+0, 0, 0, 0)
+fails with
+.BR EINVAL ,
+then this feature is definitely unsupported or disabled,

I guess it's outside the scope of the prctl.2 to describe how the
feature was disabled (e.g. sysctl).

Apart from some minor nits above, the documentation looks fine.

Thanks for writing this up.

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel