On 18/10/2022 22:15, Beata Michalska wrote:
On Thu, Oct 13, 2022 at 03:36:04PM +0200, Kevin Brodsky wrote:
On 11/10/2022 21:56, Beata Michalska wrote:
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 ?
It is indeed, this interface is completely generic.
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.
I did ponder on this too. I didn't really like introducing new flags for that reason. However, I am quite concerned that using PROT_*, or even worse VM_*, could suggest these flags have something to do with the memory the pointer refers to (i.e. whether memory mappings are configured in such a way that say a read can be made if PROT_READ is passed), when in fact this is entirely unrelated. These flags are about whether the pointer allows reading/writing, no more, no less. I feel that being explicit is important here as the overlay of capability permissions on top of MMU permissions can create a lot of confusion.
I kinda see where you are going with that. Do you forsee those being used more widely beyond what has been suggested here ? I'm just wondering if this could be solved by specifying right handlers instead of having those additional flags like: make_privileged_user_ptr_read/write/rw ?
which would be simple wrappers around make_privileged_user_ptr providing expected permission bits (skipping the naming for the time being) ? This would allow us to drop yet another abstraction on permissions and the additional typedef, and could potentially be more suggestive as per what do we mean by permissions here ?
I quite like this actually, this avoids introducing these new flags that are unlikely to be used by any other function. Will ponder some more but I would probably go for that, thanks for the suggestion!
I know I am being extra peaky here (and apologies for that), but for pointer-type capabilities CHERI permissions do end up determining what memory access is allowed. And the functionality here does consider pointer-type capabilities only, so the additional abstraction level does seem superficial.
Aside: what is the use case for having capability permissions being wider than memory access ones ? (apologies if there is an obvious answer that I am missing at this point, apparently I already got confused by CHERI permissions vs MMU ones)
Having wider capability permissions than MMU permissions is indeed unusual but is made possible by the new PROT_MAX() mechanism. An important difference is that capability permissions can never be increased, while MMU permissions are flexible. The typical example is a JIT writing code: the mapping is initially RW, then changed to RX, but X cannot be added to capability permissions so the capability needs to be RWX initially.
Something I wanted to insist on here is that these functions do not interact with mappings or PTEs at all, so for instance there is no concern with races. They are effectively pure functions (if you consider that the root capability is a constant).
Overall, my rambling here might be just that - pure rambling.
Definitely useful rambling :)
Kevin