On 22/12/2022 18:23, Zachary Leaf wrote:
as_user_ptr() is intended to be used where an arbitrary integer e.g. an error code is stored in a __user ptr.
The current implementation can be somewhat confusing in that it looks like it can be used as direct replacement for u64_to_user_ptr e.g. in PCuABI, where u64 addresses in the kernel-user interface are being replaced with capability containing types such as __kernel_uintptr_t.
Currently, passing a valid capability e.g. a __kernel_uintptr_t, __uintcap_t or user_uintptr_t etc to as_user_ptr() will result in a cast to (void __user *) and a valid capability/pointer that can be dereferenced. This is contrary to the code comment and intended usage for as_user_ptr().
Add a cast to (u64) before the cast to (void __user *)(user_uintptr_t) to make clearer the intended usage. This also always results in a null capability that cannot be dereferenced.
Signed-off-by: Zachary Leaf zachary.leaf@arm.com
rendered version: https://git.morello-project.org/zdleaf/linux/-/blob/dev/as_user_ptr/Document...
Documentation/core-api/user_ptr.rst | 22 +++++++++++++++++----- include/linux/user_ptr.h | 6 +++--- 2 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/Documentation/core-api/user_ptr.rst b/Documentation/core-api/user_ptr.rst index 21e02d4bd11b..b4b9afe88095 100644 --- a/Documentation/core-api/user_ptr.rst +++ b/Documentation/core-api/user_ptr.rst @@ -31,7 +31,8 @@ errors are likely to occur in PCuABI if it is omitted. In certain situations, it is more convenient to represent user pointers as integers. The type ``user_uintptr_t`` must be used for that purpose. It is **the only integer type** that may be directly cast to and from a -user pointer, for instance ``user_uintptr_t u = (user_uintptr_t)uptr``. +user pointer, for instance ``user_uintptr_t uint = (user_uintptr_t)uptr`` +or ``void __user *uptr = (void __user *)uint``.
Note that ``(u)intptr_t`` is the recommended type to represent kernel pointers, but it cannot represent user pointers. @@ -106,6 +107,13 @@ Each function covers a particular category of input integer:
- Integer of any type: ``as_user_ptr()``
- ``u64`` (deprecated): ``u64_to_user_ptr()``
+Note: ``as_user_ptr()`` nullifies any capability and is not a +replacement for all uses of ``u64_to_user_ptr()``. To convert an integer
I would say s/all/most/ as really as_user_ptr() almost always replaces a cast in existing code.
+representation of a user pointer i.e. user_uintptr_t back to pointer
``user_uintptr_t``
+type, a simple cast such as ``(void __user *)`` is sufficient. See +`Representing user pointers`_ and notes for ``as_user_ptr()`` and +``u64_to_user_ptr()`` below.
These functions are available in ``<linux/user_ptr.h>``, except ``compat_ptr()`` (``<linux/compat.h>``).
@@ -142,16 +150,20 @@ derived from in the PCuABI case. | | | ``compat_*`` struct | | | +------------------------------+--------------------+------------------------+-----------------------------------+------------------------------------------------------+ | ``as_user_ptr()`` | Arbitrary integer | Error code | Null capability | This is a pure representation change, as suggested | -| | | | | by the ``as_`` prefix. The resulting pointer cannot | -| | | | | be dereferenced. | +| | | | | by the ``as_`` prefix. Returns up to 64 bits of an | +| | | | | arbitrary integer represented as a user pointer. The | +| | | | | result is not a valid pointer and cannot be | +| | | | | dereferenced. | +------------------------------+--------------------+------------------------+-----------------------------------+------------------------------------------------------+ | ``u64_to_user_ptr()`` | ``u64`` integer | [Deprecated] | Null capability | Legacy function, new callers should not be added. | | | | | | Existing callers should move to either | | | | | | ``as_user_ptr()`` if the user pointer is not used to | | | | | | access memory, or ``uaddr_to_user_ptr()`` if the | | | | | | input is an address and the user pointer is | -| | | | | dereferenced (or ideally removed if the uapi can be | -| | | | | changed appropriately). | +| | | | | dereferenced (or ideally replace u64 with a | +| | | | | capability containing type such as | +| | | | | ``__kernel_uintptr_t`` if the uapi can be changed | +| | | | | appropriately). |
I think this might not be the best place to start talking about u64 -> __kernel_uintptr_t conversions, as this applies equally to uaddr_to_user_ptr(). I'm not sure how much we want to elaborate here, this could potentially belong to the porting guide instead.
Looking good otherwise!
Kevin
+------------------------------+--------------------+------------------------+-----------------------------------+------------------------------------------------------+
diff --git a/include/linux/user_ptr.h b/include/linux/user_ptr.h index 0942b58cfb6a..516024f3fead 100644 --- a/include/linux/user_ptr.h +++ b/include/linux/user_ptr.h @@ -12,10 +12,10 @@
- as_user_ptr - convert an arbitrary integer value to a user pointer
- @x: the integer value to convert
- Returns @x represented as a user pointer. The result is not a valid pointer
- and shall not be dereferenced.
- Returns up to 64 bits of @x represented as a user pointer. The result is
*/
- not a valid pointer and shall not be dereferenced.
-#define as_user_ptr(x) ((void __user *)(user_uintptr_t)(x)) +#define as_user_ptr(x) ((void __user *)(user_uintptr_t)(u64)(x))
/* Same semantics as as_user_ptr(), but also requires x to be of a given type */
#define as_user_ptr_strict(type, x) ( \
2.34.1
linux-morello mailing list -- linux-morello@op-lists.linaro.org To unsubscribe send an email to linux-morello-leave@op-lists.linaro.org