On 20/12/2022 13:20, 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 represented as an integer e.g. any
In fact this is true for any capability, regardless of how it's represented (e.g. void * __capability as well).
__kernel_uintptr_t, __uintcap_t or user_uintptr_t 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
Documentation/core-api/user_ptr.rst | 6 +++--- include/linux/user_ptr.h | 5 +++-- 2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/Documentation/core-api/user_ptr.rst b/Documentation/core-api/user_ptr.rst index 21e02d4bd11b..e4cc0676a92b 100644 --- a/Documentation/core-api/user_ptr.rst +++ b/Documentation/core-api/user_ptr.rst @@ -103,7 +103,7 @@ Each function covers a particular category of input integer:
- **Arbitrary integer**:
- Integer of any type: ``as_user_ptr()``
- 64-bit or less integer, of any type: ``as_user_ptr()``
I'm not sure about this change and the related ones below. With the new implementation of as_user_ptr(), if you pass a user_uintptr_t (for instance), the right thing will still happen (i.e. a null-derived capability with the 64-bit value of the input). This may actually be used in practice, for instance the third argument of ioctl handlers is represented as user_uintptr_t, but most of the time it really is a 64-bit integer - user_uintptr_t having become the new catch-all user type - and you may want to get an (invalid) void __user * out of it.
What we could potentially say is that "user-pointer-carrying" integers should simply be cast to void __user *, without using any helper. This is what the "Representing user pointers" section already suggests, but we could reiterate here.
- ``u64`` (deprecated): ``u64_to_user_ptr()``
These functions are available in ``<linux/user_ptr.h>``, except @@ -141,8 +141,8 @@ derived from in the PCuABI case. | | | user-provided | | pointer to a native user pointer. | | | | ``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 | +| ``as_user_ptr()`` | Arbitrary 64-bit | Error code | Null capability | This is a pure representation change, as suggested | +| | or less integer | | | by the ``as_`` prefix. The resulting pointer cannot | | | | | | be dereferenced. | +------------------------------+--------------------+------------------------+-----------------------------------+------------------------------------------------------+ | ``u64_to_user_ptr()`` | ``u64`` integer | [Deprecated] | Null capability | Legacy function, new callers should not be added. | diff --git a/include/linux/user_ptr.h b/include/linux/user_ptr.h index 0942b58cfb6a..183e40ccc51f 100644 --- a/include/linux/user_ptr.h +++ b/include/linux/user_ptr.h @@ -9,13 +9,14 @@ #endif /**
- as_user_ptr - convert an arbitrary integer value to a user pointer
- as_user_ptr - convert an arbitrary 64-bit or less 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
I think we could clarify by saying "the integer value of @x" (covering the special case of a __uintcap_t input).
Kevin
- 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) ( \