On 10/13/22 18:20, Kevin Brodsky wrote:
On 10/10/2022 11:43, Amit Kachhap wrote:
On 9/28/22 21:01, Kevin Brodsky wrote:
Introduce new helpers to operate on capability metadata in PCuABI:
- make_privileged_user_ptr() creates a user capability with the
requested bounds and permissions.
- check_user_ptr() checks that the capability is valid and its
bounds and permissions are appropriate for the requested access.
In the PCuABI case, these functions are implemented on top of the new cheri_* helpers in linux/cheri.h. The implementation is trivial in the !PCuABI case (the provided length and permissions are ignored).
make_privileged_user_ptr() supersedes uaddr_to_user_ptr_safe() as it creates safer capabilities for the kernel to manipulate.
Note that check_user_ptr() is only meant to be used when explicit checking is actually required. In particular, it is not needed when a user pointer is passed to uaccess functions and otherwise treated as an opaque value, which is the vast majority of cases.
Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com
include/linux/user_ptr.h | 64 ++++++++++++++++++++++++++++++++++++++++ lib/user_ptr.c | 33 +++++++++++++++++++++ 2 files changed, 97 insertions(+)
diff --git a/include/linux/user_ptr.h b/include/linux/user_ptr.h index 21cc0f6e1832..2a830a7f7449 100644 --- a/include/linux/user_ptr.h +++ b/include/linux/user_ptr.h @@ -4,6 +4,11 @@ #include <linux/typecheck.h> +typedef int user_ptr_perms_t;
+#define USER_PTR_CAN_READ 0x1 +#define USER_PTR_CAN_WRITE 0x2
I suppose these names are Cheri neutral as far as possible.
That's the idea yes, unrelated to CHERI in fact.
Also by looking at the Cheri Bsd, it implements coarse/block type permissions like CHERI_PERMS_KERNEL_DATA/CHERI_PERMS_KERNEL_CODE etc.
These are capability permissions, so slightly different focus. What I have introduced in cheri.h (CHERI_PERMS_*) should cover most use-cases. I'm not convinced we need the same set of macros as in CheriBSD (i.e. you can obtain what you need by combining CHERI_PERMS_* and Global), but we shall see.
Along with above we may also need coarse type permissions for binfmtelf.c.
Are you thinking about something in particular, apart from AT_CHERI_* that will clearly make use of CHERI_PERMS_*?
Probably AT_CHERI_* definitions should suffice for us. Also I suppose it can be placed in generic include/linux/elf.h or something similar.
Amit
/** * as_user_ptr() - Convert an arbitrary integer value to a user pointer. * @x: The integer value to convert. @@ -51,9 +56,56 @@ void __user *uaddr_to_user_ptr(ptraddr_t addr); * This function should be used when a user pointer is required because user * memory at a certain address needs to be accessed, and that address originates * from the kernel itself (i.e. it is not provided by userspace).
- Deprecated in favour of make_privileged_user_ptr(), which
provides more
- precise information for the pure-capability uABI.
*/ void __user *uaddr_to_user_ptr_safe(ptraddr_t addr); +/**
- make_privileged_user_ptr() - Create a user pointer from
kernel-generated
- * parameters, for in-kernel use only.
- @addr: The address to set the pointer to.
- @len: The minimum size of the region the pointer should allow to
access.
- @perms: The type of operation the pointer should allow; bitwise
combination
- * of USER_PTR_CAN_*.
- Returns a user pointer with its address set to @addr.
- This function should be used when a user pointer is required
because user
- memory at a certain address needs to be accessed, and that
address originates
- from the kernel itself. Additionally, the returned user pointer
should only
- be used by the kernel itself. In other words, the parameters
should not be
- provided by userspace, and the returned pointer should not be
provided to
- userspace in any way.
- When the pure-capability uABI is targeted, the returned
capability pointer
- will have its length set to at least @len (the base and length
may be
- expanded because of representability constraints), and its
permissions will
- be set according to @perms.
- */
+void __user *make_privileged_user_ptr(ptraddr_t addr, size_t len, + user_ptr_perms_t perms);
Minor: How about make_user_ptr_with_capability()? privileged may confuse with privilege mode or something else. Although I don't have strong opinion here.
I agree, "privileged" is a very overloaded term. What I am trying to convey is that the returned user pointer is to be used by the kernel itself only, because we do not control how the bounds are extended for representability purposes. This is fine inside the kernel as we generally cannot do better (and have enough privilege to access any part of the address space), but it is not if userspace gets hold of this capability, as it could give access to arbitrary objects in addition to the one we intended. Additionally, we provide all possible Load/Store capability permissions as requested, which is again fine when the kernel controls the access but not necessarily if userspace does (e.g. we may want to provide Load but not LoadCap).
It is in this sense that the user pointer is "privileged" (because only the kernel should use it), but I agree that word is misleading. Maybe this is too subtle to convey in the naming. Since there are very few places in which the kernel creates user pointers for userspace (I think just binfmt_elf + mmap & co, where the CHERI API should be used directly instead of this one), the risks of accidental use of this function may be low enough that we can keep the name simple. Definitely looking for opinions on this!
+/**
- check_user_ptr() - Check whether a user pointer grants access to
a memory
- * region.
- @ptr: The pointer to check.
- @len: The size of the region that needs to be accessible.
- @perms: The type of operation the pointer needs to allow; bitwise
combination
- * of USER_PTR_CAN_*.
- Returns whether @ptr allows accessing the memory region starting
at the
- address of @ptr and of size @len. The type of access that @ptr
should allow
- is specified by @perms.
- This function only checks whether the **pointer itself** allows a
given
- access; no other check is performed. Such checks are only
performed when user
- pointers have appropriate metadata, as in the pure-capability
uABI, otherwise
- the function always returns true.
- */
+bool check_user_ptr(void __user *ptr, size_t len, user_ptr_perms_t perms);
How about check_privileged_user_ptr() ? This will make it consistent like above.
This is actually the opposite situation here: the user pointer comes from userspace and we want to check it, there is nothing privileged about the pointer (it is completely untrusted).
Kevin
Thanks, Amit
#else /* CONFIG_CHERI_PURECAP_UABI */ static inline void __user *uaddr_to_user_ptr(ptraddr_t addr) @@ -66,6 +118,18 @@ static inline void __user *uaddr_to_user_ptr_safe(ptraddr_t addr) return as_user_ptr(addr); } +static inline void __user *make_privileged_user_ptr(ptraddr_t addr, size_t len, + user_ptr_perms_t perms) +{ + return as_user_ptr(addr); +}
+static inline bool check_user_ptr(void __user *ptr, size_t len, + user_ptr_perms_t perms) +{ + return true; +}
#endif /* CONFIG_CHERI_PURECAP_UABI */ /** diff --git a/lib/user_ptr.c b/lib/user_ptr.c index 503a8369d019..826996cd45bd 100644 --- a/lib/user_ptr.c +++ b/lib/user_ptr.c @@ -27,3 +27,36 @@ void __user *uaddr_to_user_ptr_safe(ptraddr_t addr) return ret; } +void __user *make_privileged_user_ptr(ptraddr_t addr, size_t len, + user_ptr_perms_t perms) +{ + cheri_perms_t cap_perms = CHERI_PERM_GLOBAL; + /* + * Grant all permissions in each category, e.g. loading/storing + * capabilities in addition to standard data. + */ + if (perms & USER_PTR_CAN_READ) + cap_perms |= CHERI_PERMS_READ; + if (perms & USER_PTR_CAN_WRITE) + cap_perms |= CHERI_PERMS_WRITE;
+ return cheri_build_user_cap_inexact_bounds(addr, len, cap_perms); +}
+bool check_user_ptr(void __user *ptr, size_t len, user_ptr_perms_t perms) +{ + cheri_perms_t cap_perms = 0; + /* + * Only check whether the capability has the minimal data permissions + * (Load / Store). The underlying assumption is that this function is + * only used before user data pages are grabbed via GUP and the data is + * then copied through a kernel mapping, and does not contain + * capabilities. + */ + if (perms & USER_PTR_CAN_READ) + cap_perms |= CHERI_PERM_LOAD; + if (perms & USER_PTR_CAN_WRITE) + cap_perms |= CHERI_PERM_STORE;
+ return cheri_check_cap_data_access(ptr, len, cap_perms); +}