On 28/12/2022 17:03, Kevin Brodsky wrote:

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.

Ack

...

+representation of a user pointer i.e. user_uintptr_t back to pointer

``user_uintptr_t``

Done

...

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

Right, maybe it's not the right place. I've removed that part here for now.

Thanks, Zach

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