On 11/10/2022 22:08, Beata Michalska wrote:
On Wed, Sep 28, 2022 at 04:31:43PM +0100, Kevin Brodsky wrote:
For architectures implementing CHERI capabilities, introduce a new cheri.h header that defines a few helpers and macros, in addition to what is available in the compiler-provided <cheriintrin.h>.
This header is only meant for code manipulating CHERI capabilities explicitly and expands to nothing if __CHERI__ is not defined.
CONFIG_ARCH_HAS_CHERI_H is also added to allow architectures to override some of the definitions by providing their own asm/cheri.h.
Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com
arch/Kconfig | 3 ++ include/linux/cheri.h | 122 ++++++++++++++++++++++++++++++++++++++++++ lib/Makefile | 2 + lib/cheri.c | 67 +++++++++++++++++++++++ 4 files changed, 194 insertions(+) create mode 100644 include/linux/cheri.h create mode 100644 lib/cheri.c
diff --git a/arch/Kconfig b/arch/Kconfig index afde8a4eeee6..b173a07c5a91 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -1376,6 +1376,9 @@ config DYNAMIC_SIGFRAME config HAVE_ARCH_NODE_DEV_GROUP bool +config ARCH_HAS_CHERI_H
- bool
I do appreciate that we did start with the ARCH_HAS_USER_PTR_H and current, ARCH_HAS_CHERI_H, seems like a natural follow-up on that one, though I am somewhat leaning towards switching that to HAVE_ARCH_CHERI_H to align with already existing (prior to CHERI changes) HAVE_ARCH_COMPILER_H - which does seem to expose same behaviour as the one we are after here. Not to mention we are getting rid of ARCH_HAS_USER_PTR_H. Or is there (maybe not so subtle) difference between ARCH_HAS_xxx and HAVE_ARCH_xxx ?
Honestly, no idea, I just picked ARCH_HAS_* because I liked the sound of it better :D arch/Kconfig has a bunch of both, there used to be ARCH_HAS_DMA_COHERENCE_H but it got removed last year. No concern with using HAVE_ARCH_COMPILER_H if you prefer it.
config ARCH_HAS_USER_PTR_H bool diff --git a/include/linux/cheri.h b/include/linux/cheri.h new file mode 100644 index 000000000000..aa03edeaeaf9 --- /dev/null +++ b/include/linux/cheri.h @@ -0,0 +1,122 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +#ifndef _LINUX_CHERI_H +#define _LINUX_CHERI_H
+#ifdef __CHERI__
+#include <cheriintrin.h>
+#include <linux/types.h>
+#include <uapi/asm/cheri.h> +#ifdef CONFIG_ARCH_HAS_CHERI_H +#include <asm/cheri.h> +#endif
+/*
- Standard permission sets for new capabilities. Can be overridden by
- architectures to add arch-specific permissions.
- */
+#ifndef CHERI_PERMS_READ +#define CHERI_PERMS_READ \
- (CHERI_PERM_LOAD | CHERI_PERM_LOAD_CAP)
+#endif
+#ifndef CHERI_PERMS_WRITE +#define CHERI_PERMS_WRITE \
- (CHERI_PERM_STORE | CHERI_PERM_STORE_CAP | CHERI_PERM_STORE_LOCAL_CAP)
+#endif
+#ifndef CHERI_PERMS_EXEC +#define CHERI_PERMS_EXEC \
- (CHERI_PERM_EXECUTE | CHERI_PERM_SYSTEM_REGS)
+#endif
+#ifndef CHERI_PERMS_ROOTCAP +#define CHERI_PERMS_ROOTCAP \
- (CHERI_PERM_GLOBAL | CHERI_PERM_SW_VMEM)
+#endif
What kind of variations of those are we expecting? Wouldn't it suffice to have them suffixed with smth like '_BASE' and defined unconditionally ?
No variations expected on the base sets, I was just trying to keep it simple. I could introduce new base sets that each arch can reuse when overriding, though I'm slightly concerned it could be misunderstood by users of cheri.h (because these base sets should never be used apart from the very specific use-case of arch overrides). Maybe prefixed with __? Not sure.
+/**
- cheri_build_user_cap() - Create a userspace capability.
- @addr: Requested capability address.
- @len: Requested capability length.
- @perms: Requested capability permissions.
- Returns a new capability derived from the root userspace capability. Its
Minor: As we intend(?) to align with the Documentation/doc-guide/kernel-doc.rst this one should probably be: Returns: A new capability ...... Same applies to the remaining ones.
Done while looking at patch 2.
- address and permissions are set according to @addr and @perms respectively.
- Its bounds are set exactly with @addr as base address and @len as length.
- The caller is responsible to ensure that:
- @addr is a valid userspace address.
- The (@addr, @len) tuple can be represented as capability bounds.
- @perms are valid permissions for a userspace capability.
- If either 1. or 2. does not hold, the resulting capability will be invalid.
- If 3. does not hold, the returned capability will not have any of the invalid
- permissions.
- */
+void * __capability +cheri_build_user_cap(ptraddr_t addr, size_t len, cheri_perms_t perms);
+/**
- cheri_build_user_cap_inexact_bounds() - Create a userspace capability,
- allowing bounds to be enlarged.
- @addr: Requested capability address.
- @len: Requested capability length.
- @perms: Requested capability permissions.
- Returns a new capability derived from the root userspace capability. Its
- address and permissions are set according to @addr and @perms respectively.
- Its bounds are set to the smallest representable range that includes the
- range [@addr, @addr + @len[.
- This variant of cheri_build_user_cap() should only be used when it is safe to
- enlarge the bounds of the capability. In particular, it should never be used
- when creating a capability that is to be provided to userspace, because the
- potentially enlarged bounds might give access to unrelated objects.
- The caller is responsible to ensure that:
- @addr is a valid userspace address.
- @perms are valid permissions for a userspace capability.
- If 1. does not hold, the resulting capability will be invalid.
- If 2. does not hold, the returned capability will not have any of the invalid
- permissions.
- */
+void * __capability +cheri_build_user_cap_inexact_bounds(ptraddr_t addr, size_t len,
cheri_perms_t perms);How about flipping the naming a bit here and have this one as : cheri_build_user_cap and the one above as cheri_build_user_cap_exact, to align with what compiler is doing with the cheri builtins (namely __builtin_cheri_bounds_set and __builtin_cheri_bounds_set_exact) ?
It would be less wonky for sure, but I did this entirely on purpose. I want to make it crystal clear that code is using the inexact variant, because this is the dangerous one from the perspective of the capability model: it can return a capability whose bounds are larger than the object you're intending to give access to, which is almost never fine (without careful processing) if the capability is then returned to userspace. That's the same rationale as for the naming of make_privileged_user_ptr() though I'll come back to this in patch 8.
+/**
- cheri_check_cap_data_access() - Check whether a capability gives access to a
- range of addresses.
- @cap: Capability to check.
- @len: Length of the access.
- @perms: Required permissions.
- Returns true if the capability gives access to a given range of addresses
- and has the requested permissions. This means that:
- @cap is valid and unsealed.
- The range [@cap.address, @cap.address + @len[ is within the bounds
- of @cap.
- The permissions of @cap include at least @perms.
- */
+bool cheri_check_cap_data_access(void * __capability cap, size_t len,
cheri_perms_t perms);Wouldn't cheri_check_cap_access be sufficient ? Or cheri_check_access even, as it is somewhat obvious it will be operating on a capability ? On a second thought, as there is access_ok already, how about simply cheri_access_ok ?
See my reply to Amit on this same patch regarding "data". Agreed "cap" is somewhat obvious, though I would only remove it if "data" is removed too as this looks a bit ambiguous to me otherwise. I would avoid cheri_access_ok() as it opens the can of worms of "why don't you just do this in access_ok()" (while here we only need this function when we cannot just dereference the pointer and let it fault).
+/*
- Root capabilities. Should be set in arch code during the early init phase,
- read-only after that.
- The helpers above should be used instead where possible.
- */
+extern uintcap_t cheri_root_cap_userspace; +extern uintcap_t cheri_root_seal_cap_userspace; +extern uintcap_t cheri_root_cid_cap_userspace;
For some reason (and those are just personal preferences here) I am not entirely convinced with the naming here: maybe cheri_user_root_xxx instead of appending the 'userspace' ? Although not much of a difference there in the end.
I did ponder on this too and had a hard time making a decision. In the end I tried to build the name in a hierarchical way, that is: "cheri" (module) + "root*_cap" (nature of the object) + "userspace" (which object exactly). We will probably end up with kernel variants too, so the question is also which of "cheri_root_cap_kernel" or "cheri_kernel_root_cap" makes more sense. Maybe the latter just looks better?
Also, it took me a moment to translate cid to compartment ID :)
Also hesitated on this one but I just aligned it on AT_CHERI_CID_CAP - cheri_root_compartmentid_cap_userspace would be quite a mouthful!
Kevin
BR B.
+#endif /* __CHERI__ */
+#endif /* _LINUX_CHERI_H */ diff --git a/lib/Makefile b/lib/Makefile index 6b9ffc1bd1ee..8dd8c00fee31 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -263,6 +263,8 @@ obj-$(CONFIG_MEMREGION) += memregion.o obj-$(CONFIG_STMP_DEVICE) += stmp_device.o obj-$(CONFIG_IRQ_POLL) += irq_poll.o +obj-y += cheri.o
- # stackdepot.c should not be instrumented or call instrumented functions. # Prevent the compiler from calling builtins like memcmp() or bcmp() from this # file.
diff --git a/lib/cheri.c b/lib/cheri.c new file mode 100644 index 000000000000..4fe8e9e7f72c --- /dev/null +++ b/lib/cheri.c @@ -0,0 +1,67 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +#ifdef __CHERI__
+#include <linux/bug.h> +#include <linux/cheri.h> +#include <linux/mm.h>
+uintcap_t cheri_root_cap_userspace __ro_after_init; +uintcap_t cheri_root_seal_cap_userspace __ro_after_init; +uintcap_t cheri_root_cid_cap_userspace __ro_after_init;
+static void * __capability +build_user_cap(ptraddr_t addr, size_t len, cheri_perms_t perms, bool exact_bounds) +{
- void * __capability ret = (void * __capability)cheri_root_cap_userspace;
- cheri_perms_t root_perms = cheri_perms_get(ret);
- ret = cheri_perms_and(ret, perms);
- ret = cheri_address_set(ret, addr);
- if (exact_bounds)
ret = cheri_bounds_set_exact(ret, len);- else
ret = cheri_bounds_set(ret, len);- WARN(perms & ~root_perms,
"Permission mask %#lx discarded while creating user capability %#lp\n",perms & ~root_perms, ret);- WARN(cheri_is_invalid(ret),
"Invalid user capability created: %#lp (%s bounds requested)\n",ret, (exact_bounds ? "exact" : "inexact"));- return ret;
+}
+void * __capability +cheri_build_user_cap(ptraddr_t addr, size_t len, cheri_perms_t perms) +{
- return build_user_cap(addr, len, perms, true);
+}
+void * __capability +cheri_build_user_cap_inexact_bounds(ptraddr_t addr, size_t len,
cheri_perms_t perms)+{
- return build_user_cap(addr, len, perms, false);
+}
+bool cheri_check_cap_data_access(void * __capability cap, size_t len,
cheri_perms_t perms)+{
- ptraddr_t addr = untagged_addr(cheri_address_get(cap));
- ptraddr_t base = cheri_base_get(cap); /* Never tagged */
- if (cheri_is_invalid(cap) || cheri_is_sealed(cap))
return false;- if (addr < base || addr > base + cheri_length_get(cap) - len)
return false;- if (perms & ~cheri_perms_get(cap))
return false;- return true;
+}
+#endif /* __CHERI__ */
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
linux-morello mailing list -- linux-morello@op-lists.linaro.org To unsubscribe send an email to linux-morello-leave@op-lists.linaro.org