On Wed, Sep 28, 2022 at 04:31:47PM +0100, 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'm probably missing smth but why do we need those additional permissions on top of capability ones? Is it about simply abstracting those ? If so, why we cannot stick to PROT_READ / PROT_WRITE (or VM_READ / VM_WRITE)? We are getting another layer of translation and I am bit concerned that this might get confusing at some point. And what about execute permission?
/**
- 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);
I'm having mixed feelings about the naming here: 'make_privileged' is somehow suggesting that we grant some rights that otherwise are not there, and this one seems to be simply creating a valid capability based on requested access, which, as I assume, should be already in place for the memory at hand. So what we are doing here is basically mapping memory access permissions to capability metadata. So it's not increasing any privileges per se, just providing different view on those. Unless, I have completely misunderstood the intention here.
+/**
- 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);
#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;
I think it would be nice to have a dedicated function for mapping whatever high-level permissions we decide to use to CHERI capability permissions.
- return cheri_check_cap_data_access(ptr, len, cap_perms);
+}
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