Hi,
This series is a follow-up to the RFC "New CHERI API and rehauled user_ptr.h", with a slightly different scope to make it more self-contained.
There are two main focuses for this series: 1. Introducing linux/cheri.h. There is no fundamental change compared to v1 here. 2. Deriving all capabilities from an appropriate userspace root capability (cheri_user_root_*) instead of morello_root_cap. v1 started this by reimplementing uaddr_to_user_ptr*, this series finishes up the work.
The focus of v1, adding generic functions to linux/user_ptr.h, has been dropped and will reappear in a separate series.
Some more details on the choice of root capabilities (see the comment in patch 5 regarding cheri_user_root_*):
* In purecap, the PCuABI spec gives us good guidance on which root capability we should use where. Namely:
- cheri_user_root_cap for almost all capabilities. The permissions correspond to the maximum permissions obtainable via mmap(). As we progress through the second phase, the bounds/permissions of capabilities derived from this root will be restricted as specified, and DDC will be set to null.
- cheri_user_root_{seal,cid}_cap for the AT_CHERI_{SEAL,CID}_CAP. These capabilities exist precisely because their permissions (Seal/Unseal/CompartmentID) are not provided in regular capabilities (derived from cheri_user_root_cap).
- cheri_user_root_all_cap for capabilities created via (privileged) ptrace. See patch 13 for some details on this.
* In hybrid, the de facto ABI is what Documentation/arm64/morello.rst says. As there is no mechanism to obtain special permissions, all capabilities are derived from cheri_user_root_all_cap. The documentation is updated accordingly.
This series introduces functional changes by restricting the bounds/permissions of all userspace capabilities, but these restrictions should not affect any valid use-case. More specifically:
* In purecap, the bounds of all capabilities are restricted to the user address space. See above for details on permissions.
* In hybrid, the bounds of capabilities are also restricted to the user address space. All relevant permissions remain available. CSP is no longer initialised to a valid capability, as this is neither required nor documented.
More detailed changelog below.
v1..v2: * Addressing review comments: - Reformatted the function documentation to make kernel-doc -v (mostly) happy. - Added some comment clarifying what CHERI_PERM_SW_VMEM is about. - Renamed ARCH_HAS_CHERI_H to HAVE_ARCH_CHERI_H. - Renamed cheri_root*_cap_userspace to cheri_user_root_*cap and added some description of each. - Renamed cheri_check_cap_data_access() to cheri_check_cap(). * New patches: - Derive compat_ptr() from cheri_user_root_all_cap (deriving from DDC proved more complicated than expected, created a ticket for that [1]) - Derive AT_CHERI_{SEAL,CID}_CAP from cheri_user_root_{seal,cid}_cap - Initialisation of capability registers from cheri_user_root_* (with a clear separation between purecap and hybrid) - Capabilities created via (privileged) ptrace now derived from cheri_user_root_all_cap - Remove morello_root_cap (no longer used) - Update documentation to reflect cheri_user_root_all_cap being the new root capability in hybrid * Other changes: - As per a recent update to the PCuABI spec, the BranchSealedPair is no longer part of the rootcap permission set. It is still needed in certain user capabilities, so moved it from CHERI_PERMS_ROOTCAP to explicit addition to cheri_user_root_cap in morello.c. - Added cheri_user_root_all_cap, the "root of roots" with all permissions. cheri_user_root_cid_cap is now derived from it too, so its bounds are not the whole address space any more. - Patch 8/9 (new functions in user_ptr.h) dropped. - Rebased on next.
Review branch:
https://git.morello-project.org/kbrodsky-arm/linux/-/commits/cheri_ptr_api_v...
Thanks, Kevin
[1] https://git.morello-project.org/morello/kernel/linux/-/issues/40
Kevin Brodsky (15): pps: Add missing #include linux/user_ptr.h: Remove kaddr_to_user_ptr() linux/user_ptr.h: Improve comment formatting arm64: uapi: Add asm/cheri.h linux/cheri.h: Introduce CHERI helpers arm64: morello: Implement cheri.h fs/binfmt_elf: Use appropriate caps for AT_CHERI_{SEAL,CID}_CAP arm64: compat: Use appropriate root cap in compat_ptr() in PCuABI linux/user_ptr.h: Generic PCuABI impl for uaddr_to_user_ptr* arm64: Remove asm/user_ptr.h arm64: morello: Initialise user capabilities from cheri_user_root_* arm64: morello: Initialise user DDC from cheri_user_root_* arm64: morello: Build arbitrary user caps using appropriate root arm64: morello: Remove morello_root_cap arm64: morello: Update root capability in documentation
Documentation/arm64/morello.rst | 23 +++-- Documentation/core-api/user_ptr.rst | 8 -- arch/Kconfig | 2 +- arch/arm64/Kconfig | 2 +- arch/arm64/include/asm/cheri.h | 11 +++ arch/arm64/include/asm/compat.h | 9 +- arch/arm64/include/asm/morello.h | 12 ++- arch/arm64/include/asm/user_ptr.h | 43 --------- arch/arm64/include/uapi/asm/cheri.h | 11 +++ arch/arm64/kernel/morello.c | 143 +++++++++++++++++----------- arch/arm64/kernel/process.c | 2 +- arch/arm64/kernel/ptrace.c | 2 +- arch/arm64/lib/morello.S | 17 ++-- drivers/pps/pps.c | 1 + fs/binfmt_elf.c | 10 +- include/linux/cheri.h | 132 +++++++++++++++++++++++++ include/linux/user_ptr.h | 69 ++++++-------- lib/Makefile | 3 + lib/cheri.c | 72 ++++++++++++++ lib/user_ptr.c | 26 +++++ 20 files changed, 413 insertions(+), 185 deletions(-) create mode 100644 arch/arm64/include/asm/cheri.h delete mode 100644 arch/arm64/include/asm/user_ptr.h create mode 100644 arch/arm64/include/uapi/asm/cheri.h create mode 100644 include/linux/cheri.h create mode 100644 lib/cheri.c create mode 100644 lib/user_ptr.c
Commit "pps: Update compat ioctl handler for compat64" introduced the use of compat_ptr() in pps_cdev_compat_ioctl(), which relied on the implicit inclusion of <asm/compat.h>. compat_ptr() is generally defined in the generic <linux/compat.h>, so make sure to include it explicitly.
Fixes: ("pps: Update compat ioctl handler for compat64") Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com ---
This is to avoid breaking the build in !PCuABI after patch 8.
drivers/pps/pps.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/pps/pps.c b/drivers/pps/pps.c index 45551c113172..dba5a67a92d1 100644 --- a/drivers/pps/pps.c +++ b/drivers/pps/pps.c @@ -18,6 +18,7 @@ #include <linux/poll.h> #include <linux/pps_kernel.h> #include <linux/slab.h> +#include <linux/compat.h>
#include "kc.h"
This one does not seem to be tightly coupled with the rest of the patches within the series so maybe pull it out and send separately as I guess it could be already merged ?
--- BR B. On Mon, Dec 12, 2022 at 10:35:36AM +0000, Kevin Brodsky wrote:
Commit "pps: Update compat ioctl handler for compat64" introduced the use of compat_ptr() in pps_cdev_compat_ioctl(), which relied on the implicit inclusion of <asm/compat.h>. compat_ptr() is generally defined in the generic <linux/compat.h>, so make sure to include it explicitly.
Fixes: ("pps: Update compat ioctl handler for compat64") Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com
This is to avoid breaking the build in !PCuABI after patch 8.
drivers/pps/pps.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/pps/pps.c b/drivers/pps/pps.c index 45551c113172..dba5a67a92d1 100644 --- a/drivers/pps/pps.c +++ b/drivers/pps/pps.c @@ -18,6 +18,7 @@ #include <linux/poll.h> #include <linux/pps_kernel.h> #include <linux/slab.h> +#include <linux/compat.h> #include "kc.h" -- 2.38.1
linux-morello mailing list -- linux-morello@op-lists.linaro.org To unsubscribe send an email to linux-morello-leave@op-lists.linaro.org
On 31/01/2023 12:27, Beata Michalska wrote:
This one does not seem to be tightly coupled with the rest of the patches within the series so maybe pull it out and send separately as I guess it could be already merged ?
I think that's fair yes, if you think it looks good I would just merge it now.
Kevin
BR B. On Mon, Dec 12, 2022 at 10:35:36AM +0000, Kevin Brodsky wrote:
Commit "pps: Update compat ioctl handler for compat64" introduced the use of compat_ptr() in pps_cdev_compat_ioctl(), which relied on the implicit inclusion of <asm/compat.h>. compat_ptr() is generally defined in the generic <linux/compat.h>, so make sure to include it explicitly.
Fixes: ("pps: Update compat ioctl handler for compat64") Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com
This is to avoid breaking the build in !PCuABI after patch 8.
drivers/pps/pps.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/pps/pps.c b/drivers/pps/pps.c index 45551c113172..dba5a67a92d1 100644 --- a/drivers/pps/pps.c +++ b/drivers/pps/pps.c @@ -18,6 +18,7 @@ #include <linux/poll.h> #include <linux/pps_kernel.h> #include <linux/slab.h> +#include <linux/compat.h> #include "kc.h" -- 2.38.1
linux-morello mailing list -- linux-morello@op-lists.linaro.org To unsubscribe send an email to linux-morello-leave@op-lists.linaro.org
On Thu, Feb 02, 2023 at 09:32:54AM +0100, Kevin Brodsky wrote:
On 31/01/2023 12:27, Beata Michalska wrote:
This one does not seem to be tightly coupled with the rest of the patches within the series so maybe pull it out and send separately as I guess it could be already merged ?
I think that's fair yes, if you think it looks good I would just merge it now.
I'd say go ahead and merge it. Thanks!
--- BR B.
Kevin
BR B. On Mon, Dec 12, 2022 at 10:35:36AM +0000, Kevin Brodsky wrote:
Commit "pps: Update compat ioctl handler for compat64" introduced the use of compat_ptr() in pps_cdev_compat_ioctl(), which relied on the implicit inclusion of <asm/compat.h>. compat_ptr() is generally defined in the generic <linux/compat.h>, so make sure to include it explicitly.
Fixes: ("pps: Update compat ioctl handler for compat64") Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com
This is to avoid breaking the build in !PCuABI after patch 8.
drivers/pps/pps.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/pps/pps.c b/drivers/pps/pps.c index 45551c113172..dba5a67a92d1 100644 --- a/drivers/pps/pps.c +++ b/drivers/pps/pps.c @@ -18,6 +18,7 @@ #include <linux/poll.h> #include <linux/pps_kernel.h> #include <linux/slab.h> +#include <linux/compat.h> #include "kc.h" -- 2.38.1
linux-morello mailing list -- linux-morello@op-lists.linaro.org To unsubscribe send an email to linux-morello-leave@op-lists.linaro.org
On 02/02/2023 17:00, Beata Michalska wrote:
On Thu, Feb 02, 2023 at 09:32:54AM +0100, Kevin Brodsky wrote:
On 31/01/2023 12:27, Beata Michalska wrote:
This one does not seem to be tightly coupled with the rest of the patches within the series so maybe pull it out and send separately as I guess it could be already merged ?
I think that's fair yes, if you think it looks good I would just merge it now.
I'd say go ahead and merge it. Thanks!
Thanks, done :) Now in next.
Kevin
BR B.
Kevin
BR B. On Mon, Dec 12, 2022 at 10:35:36AM +0000, Kevin Brodsky wrote:
Commit "pps: Update compat ioctl handler for compat64" introduced the use of compat_ptr() in pps_cdev_compat_ioctl(), which relied on the implicit inclusion of <asm/compat.h>. compat_ptr() is generally defined in the generic <linux/compat.h>, so make sure to include it explicitly.
Fixes: ("pps: Update compat ioctl handler for compat64") Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com
This is to avoid breaking the build in !PCuABI after patch 8.
drivers/pps/pps.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/pps/pps.c b/drivers/pps/pps.c index 45551c113172..dba5a67a92d1 100644 --- a/drivers/pps/pps.c +++ b/drivers/pps/pps.c @@ -18,6 +18,7 @@ #include <linux/poll.h> #include <linux/pps_kernel.h> #include <linux/slab.h> +#include <linux/compat.h> #include "kc.h" -- 2.38.1
linux-morello mailing list -- linux-morello@op-lists.linaro.org To unsubscribe send an email to linux-morello-leave@op-lists.linaro.org
The removal of set_fs() is now firmly established and there should therefore be no need to create user pointers from kernel addresses any more. This means we can get rid of kaddr_to_user_ptr(), which is unused.
Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com --- Documentation/core-api/user_ptr.rst | 8 -------- arch/arm64/include/asm/user_ptr.h | 8 -------- include/linux/user_ptr.h | 16 ---------------- 3 files changed, 32 deletions(-)
diff --git a/Documentation/core-api/user_ptr.rst b/Documentation/core-api/user_ptr.rst index 21e02d4bd11b..925d22df2b9d 100644 --- a/Documentation/core-api/user_ptr.rst +++ b/Documentation/core-api/user_ptr.rst @@ -97,7 +97,6 @@ Each function covers a particular category of input integer:
- User-provided user address: ``uaddr_to_user_ptr()`` - Kernel-controlled user address: ``uaddr_to_user_ptr_safe()`` - - Kernel address: ``kaddr_to_user_ptr()``
* **Compat pointer**: ``compat_ptr()``
@@ -130,13 +129,6 @@ derived from in the PCuABI case. | | user address | mappings during | | kernel needs to access user memory using a bare | | | | process initialisation | | virtual address that is not provided by userspace. | +------------------------------+--------------------+------------------------+-----------------------------------+------------------------------------------------------+ -| ``kaddr_to_user_ptr()`` | Kernel address | [None currently] | Kernel root capability | There used to be a number of situations where kernel | -| | | | | memory was accessed through uaccess, requiring user | -| | | | | pointers to be created out of kernel addresses. | -| | | | | This should no longer be the case and this function | -| | | | | will be removed once it is confirmed that there is | -| | | | | no use-case left. | -+------------------------------+--------------------+------------------------+-----------------------------------+------------------------------------------------------+ | ``compat_ptr()`` | Compat pointer | Pointer in a | Current user DDC | Must be used whenever converting a compat user | | | | user-provided | | pointer to a native user pointer. | | | | ``compat_*`` struct | | | diff --git a/arch/arm64/include/asm/user_ptr.h b/arch/arm64/include/asm/user_ptr.h index 745e7d34f25b..323ad0301cbd 100644 --- a/arch/arm64/include/asm/user_ptr.h +++ b/arch/arm64/include/asm/user_ptr.h @@ -30,14 +30,6 @@ static inline void __user *uaddr_to_user_ptr_safe(ptraddr_t addr) } #define uaddr_to_user_ptr_safe(addr) uaddr_to_user_ptr_safe(addr)
-static inline void __user *kaddr_to_user_ptr(ptraddr_t addr) -{ - uintcap_t root_cap = morello_get_root_cap(); - - return (void __user *)__builtin_cheri_address_set(root_cap, addr); -} -#define kaddr_to_user_ptr(addr) kaddr_to_user_ptr(addr) - #endif /* CONFIG_CHERI_PURECAP_UABI */
#endif /* __ASM_USER_PTR_H */ diff --git a/include/linux/user_ptr.h b/include/linux/user_ptr.h index 0942b58cfb6a..e2de3464bcf8 100644 --- a/include/linux/user_ptr.h +++ b/include/linux/user_ptr.h @@ -65,22 +65,6 @@ static inline void __user *uaddr_to_user_ptr_safe(ptraddr_t addr) } #endif
-#ifndef kaddr_to_user_ptr -/** - * kaddr_to_user_ptr - convert a kernel address to a user pointer - * @addr: the address to set the pointer to - * - * Returns a user pointer with its address set to @addr. - * - * This function should be used when kernel memory needs to be accessed via a - * user pointer. There should be no use for it after the removal of set_fs(). - */ -static inline void __user *kaddr_to_user_ptr(ptraddr_t addr) -{ - return as_user_ptr(addr); -} -#endif - /** * user_ptr_addr - extract the address of a user pointer * @ptr: the user pointer to extract the address from
On Mon, Dec 12, 2022 at 10:35:37AM +0000, Kevin Brodsky wrote:
The removal of set_fs() is now firmly established and there should
We could actually mention the exact commit that removed support for CONFIG_SET_FS ? 967747bbc084 : uaccess: remove CONFIG_SET_FS
--- BR B.
therefore be no need to create user pointers from kernel addresses any more. This means we can get rid of kaddr_to_user_ptr(), which is unused.
Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com
Documentation/core-api/user_ptr.rst | 8 -------- arch/arm64/include/asm/user_ptr.h | 8 -------- include/linux/user_ptr.h | 16 ---------------- 3 files changed, 32 deletions(-)
diff --git a/Documentation/core-api/user_ptr.rst b/Documentation/core-api/user_ptr.rst index 21e02d4bd11b..925d22df2b9d 100644 --- a/Documentation/core-api/user_ptr.rst +++ b/Documentation/core-api/user_ptr.rst @@ -97,7 +97,6 @@ Each function covers a particular category of input integer:
- User-provided user address: ``uaddr_to_user_ptr()``
- Kernel-controlled user address: ``uaddr_to_user_ptr_safe()``
- Kernel address: ``kaddr_to_user_ptr()``
- **Compat pointer**: ``compat_ptr()``
@@ -130,13 +129,6 @@ derived from in the PCuABI case. | | user address | mappings during | | kernel needs to access user memory using a bare | | | | process initialisation | | virtual address that is not provided by userspace. | +------------------------------+--------------------+------------------------+-----------------------------------+------------------------------------------------------+ -| ``kaddr_to_user_ptr()`` | Kernel address | [None currently] | Kernel root capability | There used to be a number of situations where kernel | -| | | | | memory was accessed through uaccess, requiring user | -| | | | | pointers to be created out of kernel addresses. | -| | | | | This should no longer be the case and this function | -| | | | | will be removed once it is confirmed that there is | -| | | | | no use-case left. | -+------------------------------+--------------------+------------------------+-----------------------------------+------------------------------------------------------+ | ``compat_ptr()`` | Compat pointer | Pointer in a | Current user DDC | Must be used whenever converting a compat user | | | | user-provided | | pointer to a native user pointer. | | | | ``compat_*`` struct | | | diff --git a/arch/arm64/include/asm/user_ptr.h b/arch/arm64/include/asm/user_ptr.h index 745e7d34f25b..323ad0301cbd 100644 --- a/arch/arm64/include/asm/user_ptr.h +++ b/arch/arm64/include/asm/user_ptr.h @@ -30,14 +30,6 @@ static inline void __user *uaddr_to_user_ptr_safe(ptraddr_t addr) } #define uaddr_to_user_ptr_safe(addr) uaddr_to_user_ptr_safe(addr) -static inline void __user *kaddr_to_user_ptr(ptraddr_t addr) -{
- uintcap_t root_cap = morello_get_root_cap();
- return (void __user *)__builtin_cheri_address_set(root_cap, addr);
-} -#define kaddr_to_user_ptr(addr) kaddr_to_user_ptr(addr)
#endif /* CONFIG_CHERI_PURECAP_UABI */ #endif /* __ASM_USER_PTR_H */ diff --git a/include/linux/user_ptr.h b/include/linux/user_ptr.h index 0942b58cfb6a..e2de3464bcf8 100644 --- a/include/linux/user_ptr.h +++ b/include/linux/user_ptr.h @@ -65,22 +65,6 @@ static inline void __user *uaddr_to_user_ptr_safe(ptraddr_t addr) } #endif -#ifndef kaddr_to_user_ptr -/**
- kaddr_to_user_ptr - convert a kernel address to a user pointer
- @addr: the address to set the pointer to
- Returns a user pointer with its address set to @addr.
- This function should be used when kernel memory needs to be accessed via a
- user pointer. There should be no use for it after the removal of set_fs().
- */
-static inline void __user *kaddr_to_user_ptr(ptraddr_t addr) -{
- return as_user_ptr(addr);
-} -#endif
/**
- user_ptr_addr - extract the address of a user pointer
- @ptr: the user pointer to extract the address from
-- 2.38.1
linux-morello mailing list -- linux-morello@op-lists.linaro.org To unsubscribe send an email to linux-morello-leave@op-lists.linaro.org
On 31/01/2023 12:25, Beata Michalska wrote:
On Mon, Dec 12, 2022 at 10:35:37AM +0000, Kevin Brodsky wrote:
The removal of set_fs() is now firmly established and there should
We could actually mention the exact commit that removed support for CONFIG_SET_FS ? 967747bbc084 : uaccess: remove CONFIG_SET_FS
Right the way I thought about it was that it happened over a certain period of time, but indeed it's worth mentioning the final commit, will add that.
Kevin
BR B.
therefore be no need to create user pointers from kernel addresses any more. This means we can get rid of kaddr_to_user_ptr(), which is unused.
Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com
Documentation/core-api/user_ptr.rst | 8 -------- arch/arm64/include/asm/user_ptr.h | 8 -------- include/linux/user_ptr.h | 16 ---------------- 3 files changed, 32 deletions(-)
diff --git a/Documentation/core-api/user_ptr.rst b/Documentation/core-api/user_ptr.rst index 21e02d4bd11b..925d22df2b9d 100644 --- a/Documentation/core-api/user_ptr.rst +++ b/Documentation/core-api/user_ptr.rst @@ -97,7 +97,6 @@ Each function covers a particular category of input integer:
- User-provided user address: ``uaddr_to_user_ptr()``
- Kernel-controlled user address: ``uaddr_to_user_ptr_safe()``
- Kernel address: ``kaddr_to_user_ptr()``
- **Compat pointer**: ``compat_ptr()``
@@ -130,13 +129,6 @@ derived from in the PCuABI case. | | user address | mappings during | | kernel needs to access user memory using a bare | | | | process initialisation | | virtual address that is not provided by userspace. | +------------------------------+--------------------+------------------------+-----------------------------------+------------------------------------------------------+ -| ``kaddr_to_user_ptr()`` | Kernel address | [None currently] | Kernel root capability | There used to be a number of situations where kernel | -| | | | | memory was accessed through uaccess, requiring user | -| | | | | pointers to be created out of kernel addresses. | -| | | | | This should no longer be the case and this function | -| | | | | will be removed once it is confirmed that there is | -| | | | | no use-case left. | -+------------------------------+--------------------+------------------------+-----------------------------------+------------------------------------------------------+ | ``compat_ptr()`` | Compat pointer | Pointer in a | Current user DDC | Must be used whenever converting a compat user | | | | user-provided | | pointer to a native user pointer. | | | | ``compat_*`` struct | | | diff --git a/arch/arm64/include/asm/user_ptr.h b/arch/arm64/include/asm/user_ptr.h index 745e7d34f25b..323ad0301cbd 100644 --- a/arch/arm64/include/asm/user_ptr.h +++ b/arch/arm64/include/asm/user_ptr.h @@ -30,14 +30,6 @@ static inline void __user *uaddr_to_user_ptr_safe(ptraddr_t addr) } #define uaddr_to_user_ptr_safe(addr) uaddr_to_user_ptr_safe(addr) -static inline void __user *kaddr_to_user_ptr(ptraddr_t addr) -{
- uintcap_t root_cap = morello_get_root_cap();
- return (void __user *)__builtin_cheri_address_set(root_cap, addr);
-} -#define kaddr_to_user_ptr(addr) kaddr_to_user_ptr(addr)
#endif /* CONFIG_CHERI_PURECAP_UABI */ #endif /* __ASM_USER_PTR_H */ diff --git a/include/linux/user_ptr.h b/include/linux/user_ptr.h index 0942b58cfb6a..e2de3464bcf8 100644 --- a/include/linux/user_ptr.h +++ b/include/linux/user_ptr.h @@ -65,22 +65,6 @@ static inline void __user *uaddr_to_user_ptr_safe(ptraddr_t addr) } #endif -#ifndef kaddr_to_user_ptr -/**
- kaddr_to_user_ptr - convert a kernel address to a user pointer
- @addr: the address to set the pointer to
- Returns a user pointer with its address set to @addr.
- This function should be used when kernel memory needs to be accessed via a
- user pointer. There should be no use for it after the removal of set_fs().
- */
-static inline void __user *kaddr_to_user_ptr(ptraddr_t addr) -{
- return as_user_ptr(addr);
-} -#endif
/**
- user_ptr_addr - extract the address of a user pointer
- @ptr: the user pointer to extract the address from
-- 2.38.1
linux-morello mailing list -- linux-morello@op-lists.linaro.org To unsubscribe send an email to linux-morello-leave@op-lists.linaro.org
On Thu, Feb 02, 2023 at 09:44:08AM +0100, Kevin Brodsky wrote:
On 31/01/2023 12:25, Beata Michalska wrote:
On Mon, Dec 12, 2022 at 10:35:37AM +0000, Kevin Brodsky wrote:
The removal of set_fs() is now firmly established and there should
We could actually mention the exact commit that removed support for CONFIG_SET_FS ? 967747bbc084 : uaccess: remove CONFIG_SET_FS
Right the way I thought about it was that it happened over a certain period of time, but indeed it's worth mentioning the final commit, will add that.
I get the idea, which is good, but that was the ultimate end of things for set_fs so I thought it would be nice to mention it.
--- BR B.
Kevin
BR B.
therefore be no need to create user pointers from kernel addresses any more. This means we can get rid of kaddr_to_user_ptr(), which is unused.
Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com
Documentation/core-api/user_ptr.rst | 8 -------- arch/arm64/include/asm/user_ptr.h | 8 -------- include/linux/user_ptr.h | 16 ---------------- 3 files changed, 32 deletions(-)
diff --git a/Documentation/core-api/user_ptr.rst b/Documentation/core-api/user_ptr.rst index 21e02d4bd11b..925d22df2b9d 100644 --- a/Documentation/core-api/user_ptr.rst +++ b/Documentation/core-api/user_ptr.rst @@ -97,7 +97,6 @@ Each function covers a particular category of input integer:
- User-provided user address: ``uaddr_to_user_ptr()``
- Kernel-controlled user address: ``uaddr_to_user_ptr_safe()``
- Kernel address: ``kaddr_to_user_ptr()``
- **Compat pointer**: ``compat_ptr()``
@@ -130,13 +129,6 @@ derived from in the PCuABI case. | | user address | mappings during | | kernel needs to access user memory using a bare | | | | process initialisation | | virtual address that is not provided by userspace. | +------------------------------+--------------------+------------------------+-----------------------------------+------------------------------------------------------+ -| ``kaddr_to_user_ptr()`` | Kernel address | [None currently] | Kernel root capability | There used to be a number of situations where kernel | -| | | | | memory was accessed through uaccess, requiring user | -| | | | | pointers to be created out of kernel addresses. | -| | | | | This should no longer be the case and this function | -| | | | | will be removed once it is confirmed that there is | -| | | | | no use-case left. | -+------------------------------+--------------------+------------------------+-----------------------------------+------------------------------------------------------+ | ``compat_ptr()`` | Compat pointer | Pointer in a | Current user DDC | Must be used whenever converting a compat user | | | | user-provided | | pointer to a native user pointer. | | | | ``compat_*`` struct | | | diff --git a/arch/arm64/include/asm/user_ptr.h b/arch/arm64/include/asm/user_ptr.h index 745e7d34f25b..323ad0301cbd 100644 --- a/arch/arm64/include/asm/user_ptr.h +++ b/arch/arm64/include/asm/user_ptr.h @@ -30,14 +30,6 @@ static inline void __user *uaddr_to_user_ptr_safe(ptraddr_t addr) } #define uaddr_to_user_ptr_safe(addr) uaddr_to_user_ptr_safe(addr) -static inline void __user *kaddr_to_user_ptr(ptraddr_t addr) -{
- uintcap_t root_cap = morello_get_root_cap();
- return (void __user *)__builtin_cheri_address_set(root_cap, addr);
-} -#define kaddr_to_user_ptr(addr) kaddr_to_user_ptr(addr)
#endif /* CONFIG_CHERI_PURECAP_UABI */ #endif /* __ASM_USER_PTR_H */ diff --git a/include/linux/user_ptr.h b/include/linux/user_ptr.h index 0942b58cfb6a..e2de3464bcf8 100644 --- a/include/linux/user_ptr.h +++ b/include/linux/user_ptr.h @@ -65,22 +65,6 @@ static inline void __user *uaddr_to_user_ptr_safe(ptraddr_t addr) } #endif -#ifndef kaddr_to_user_ptr -/**
- kaddr_to_user_ptr - convert a kernel address to a user pointer
- @addr: the address to set the pointer to
- Returns a user pointer with its address set to @addr.
- This function should be used when kernel memory needs to be accessed via a
- user pointer. There should be no use for it after the removal of set_fs().
- */
-static inline void __user *kaddr_to_user_ptr(ptraddr_t addr) -{
- return as_user_ptr(addr);
-} -#endif
/**
- user_ptr_addr - extract the address of a user pointer
- @ptr: the user pointer to extract the address from
-- 2.38.1
linux-morello mailing list -- linux-morello@op-lists.linaro.org To unsubscribe send an email to linux-morello-leave@op-lists.linaro.org
Format kernel-doc comments as per the recommendations in Documentation/doc-guide/kernel-doc.rst and the output of kernel-doc -v.
Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com --- include/linux/user_ptr.h | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-)
diff --git a/include/linux/user_ptr.h b/include/linux/user_ptr.h index e2de3464bcf8..7e0278567947 100644 --- a/include/linux/user_ptr.h +++ b/include/linux/user_ptr.h @@ -9,11 +9,11 @@ #endif
/** - * as_user_ptr - convert an arbitrary integer value to a user pointer - * @x: the integer value to convert + * as_user_ptr() - Convert an arbitrary 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 - * and shall not be dereferenced. + * Return: @x represented as a user pointer. The result is not a valid pointer + * and shall not be dereferenced. */ #define as_user_ptr(x) ((void __user *)(user_uintptr_t)(x))
@@ -30,10 +30,10 @@
#ifndef uaddr_to_user_ptr /** - * uaddr_to_user_ptr - convert a user-provided address to a user pointer - * @addr: the address to set the pointer to + * uaddr_to_user_ptr() - Convert a user-provided address to a user pointer. + * @addr: The address to set the pointer to. * - * Returns a user pointer with its address set to @addr. + * Return: A user pointer with its address set to @addr. * * This function should be used when a user pointer is required because userspace * provided a raw address (e.g. via a __u64 member of a struct), and the memory @@ -50,10 +50,11 @@ static inline void __user *uaddr_to_user_ptr(ptraddr_t addr)
#ifndef uaddr_to_user_ptr_safe /** - * uaddr_to_user_ptr_safe - convert a kernel-generated user address to a user pointer - * @addr: the address to set the pointer to + * uaddr_to_user_ptr_safe() - Convert a kernel-generated user address to a + * user pointer. + * @addr: The address to set the pointer to. * - * Returns a user pointer with its address set to @addr. + * Return: 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 @@ -66,10 +67,10 @@ static inline void __user *uaddr_to_user_ptr_safe(ptraddr_t addr) #endif
/** - * user_ptr_addr - extract the address of a user pointer - * @ptr: the user pointer to extract the address from + * user_ptr_addr() - Extract the address of a user pointer. + * @ptr: The user pointer to extract the address from. * - * Returns the address @ptr points to. + * Return: The address @ptr points to. */ static inline ptraddr_t user_ptr_addr(const void __user *ptr) { @@ -77,9 +78,11 @@ static inline ptraddr_t user_ptr_addr(const void __user *ptr) }
/** - * user_ptr_is_same - checks where two user pointers are exactly the same + * user_ptr_is_same() - Checks where two user pointers are exactly the same. + * @p1: The first user pointer to check. + * @p2: The second user pointer to check. * - * Returns true if @p1 and @p2 are exactly the same user pointers. + * Return: true if @p1 and @p2 are exactly the same user pointers. * * Only use this function if you need to know that two user pointers are * interchangeable, not to check that their address is the same (use the ==
Add a new uapi header to define generic CHERI concepts with arm64-specific values.
To start with, provide a macro representing the VMem software permission, as defined in the PCuABI specification. The macro name matches CheriBSD, facilitating its use in a (somewhat) OS-agnostic way.
Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com --- arch/arm64/include/uapi/asm/cheri.h | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 arch/arm64/include/uapi/asm/cheri.h
diff --git a/arch/arm64/include/uapi/asm/cheri.h b/arch/arm64/include/uapi/asm/cheri.h new file mode 100644 index 000000000000..f08444167e64 --- /dev/null +++ b/arch/arm64/include/uapi/asm/cheri.h @@ -0,0 +1,11 @@ +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ +#ifndef _UAPI__ASM_CHERI_H +#define _UAPI__ASM_CHERI_H + +/* + * VMem software-defined capability permission, assigned to the User[0] + * permission on Morello (bit 2). + */ +#define CHERI_PERM_SW_VMEM (1 << 2) + +#endif /* _UAPI__ASM_CHERI_H */
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_HAVE_ARCH_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 | 132 ++++++++++++++++++++++++++++++++++++++++++ lib/Makefile | 2 + lib/cheri.c | 72 +++++++++++++++++++++++ 4 files changed, 209 insertions(+) create mode 100644 include/linux/cheri.h create mode 100644 lib/cheri.c
diff --git a/arch/Kconfig b/arch/Kconfig index afde8a4eeee6..6c3c2e19b77e 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -1376,6 +1376,9 @@ config DYNAMIC_SIGFRAME config HAVE_ARCH_NODE_DEV_GROUP bool
+config HAVE_ARCH_CHERI_H + bool + 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..5f95f1ac8f01 --- /dev/null +++ b/include/linux/cheri.h @@ -0,0 +1,132 @@ +/* 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_HAVE_ARCH_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 + +/** + * cheri_build_user_cap() - Create a userspace capability. + * @addr: Requested capability address. + * @len: Requested capability length. + * @perms: Requested capability permissions. + * + * Return: A new capability derived from cheri_user_root_cap. Its 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: + * 1. @addr is a valid userspace address. + * 2. The (@addr, @len) tuple can be represented as capability bounds. + * 3. @perms are valid permissions for a regular 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. + * + * Return: A new capability derived from cheri_user_root_cap. 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: + * 1. @addr is a valid userspace address. + * 2. @perms are valid permissions for a regular 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); + + +/** + * cheri_check_cap() - Check whether a capability gives access to a range of + * addresses. + * @cap: Capability to check. + * @len: Length of the access. + * @perms: Required permissions. + * + * Checks whether @cap 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. + * + * Return: true if @cap passes the checks. + */ +bool cheri_check_cap(void * __capability cap, size_t len, cheri_perms_t perms); + + +/* + * Root capabilities. Should be set in arch code during the early init phase, + * read-only after that. + * + * cheri_user_root_cap is the standard root capability to derive new regular + * (data/code) capabilities from. It does not include the special permissions + * Seal/Unseal and CompartmentID; those are available separately via + * cheri_user_root_{seal,cid}_cap. Finally cheri_user_root_all_cap includes all + * permissions accessible to userspace and is ultimately the root of all user + * capabilities; it should only be used in very specific situations. + * + * The helpers above should be used instead where possible. + */ +extern uintcap_t cheri_user_root_cap; /* Userspace (data/code) root */ +extern uintcap_t cheri_user_root_seal_cap; /* Userspace sealing root */ +extern uintcap_t cheri_user_root_cid_cap; /* Userspace compartment ID root */ +extern uintcap_t cheri_user_root_all_cap; /* Userspace root (all permissions) */ + +#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..d895e6594c21 --- /dev/null +++ b/lib/cheri.c @@ -0,0 +1,72 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +#ifdef __CHERI__ + +#include <linux/bug.h> +#include <linux/cheri.h> +#include <linux/mm.h> + +uintcap_t cheri_user_root_cap __ro_after_init; +uintcap_t cheri_user_root_seal_cap __ro_after_init; +uintcap_t cheri_user_root_cid_cap __ro_after_init; +uintcap_t cheri_user_root_all_cap __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_user_root_cap; + 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(void * __capability cap, size_t len, cheri_perms_t perms) +{ + ptraddr_t addr = untagged_addr(cheri_address_get(cap)); + /* + * The base address (as returned by cheri_base_get()) is never tagged, + * that is its top byte is always canonical, so no need for + * untagged_addr(). + */ + ptraddr_t base = cheri_base_get(cap); + + 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__ */
On Mon, Dec 12, 2022 at 10:35:40AM +0000, 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_HAVE_ARCH_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 | 132 ++++++++++++++++++++++++++++++++++++++++++ lib/Makefile | 2 + lib/cheri.c | 72 +++++++++++++++++++++++ 4 files changed, 209 insertions(+) create mode 100644 include/linux/cheri.h create mode 100644 lib/cheri.c
diff --git a/arch/Kconfig b/arch/Kconfig index afde8a4eeee6..6c3c2e19b77e 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -1376,6 +1376,9 @@ config DYNAMIC_SIGFRAME config HAVE_ARCH_NODE_DEV_GROUP bool +config HAVE_ARCH_CHERI_H
- bool
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..5f95f1ac8f01 --- /dev/null +++ b/include/linux/cheri.h @@ -0,0 +1,132 @@ +/* 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_HAVE_ARCH_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
+/**
- cheri_build_user_cap() - Create a userspace capability.
- @addr: Requested capability address.
- @len: Requested capability length.
- @perms: Requested capability permissions.
- Return: A new capability derived from cheri_user_root_cap. Its 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 regular 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);
The returned capability might be invalid as of having tag cleared. I assume in most of the cases it will be just fine, but wondering if it would make sense to return either capability with null capability , or one that could be validated with PTR_ERR somewhat ? Just thinking at loud here, how one would verify the result is valid (currently it would need check tag but maybe we could utilize more generic way of validating pointers ... On the other hand those new helpers are available only when __CHERI__ is defined so I assume there are only to be used in CHERI-related contexts ?
+/**
- 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.
- Return: A new capability derived from cheri_user_root_cap. 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 regular 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);+/**
- cheri_check_cap() - Check whether a capability gives access to a range of
addresses.
- @cap: Capability to check.
- @len: Length of the access.
- @perms: Required permissions.
- Checks whether @cap 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.
- Return: true if @cap passes the checks.
- */
+bool cheri_check_cap(void * __capability cap, size_t len, cheri_perms_t perms);
Minor but maybe void * __capability with const qualifier ?
+/*
- Root capabilities. Should be set in arch code during the early init phase,
- read-only after that.
- cheri_user_root_cap is the standard root capability to derive new regular
- (data/code) capabilities from. It does not include the special permissions
- Seal/Unseal and CompartmentID; those are available separately via
- cheri_user_root_{seal,cid}_cap. Finally cheri_user_root_all_cap includes all
- permissions accessible to userspace and is ultimately the root of all user
- capabilities; it should only be used in very specific situations.
- The helpers above should be used instead where possible.
- */
+extern uintcap_t cheri_user_root_cap; /* Userspace (data/code) root */ +extern uintcap_t cheri_user_root_seal_cap; /* Userspace sealing root */ +extern uintcap_t cheri_user_root_cid_cap; /* Userspace compartment ID root */ +extern uintcap_t cheri_user_root_all_cap; /* Userspace root (all permissions) */
xxx_root_all_cap somehwat reads to me as 'all capabilities' instead of 'capability with all permissions' even the context guards the interpretation, but maybe xxx_root_fill_cap instead ?
+#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..d895e6594c21 --- /dev/null +++ b/lib/cheri.c @@ -0,0 +1,72 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +#ifdef __CHERI__
+#include <linux/bug.h> +#include <linux/cheri.h> +#include <linux/mm.h>
+uintcap_t cheri_user_root_cap __ro_after_init; +uintcap_t cheri_user_root_seal_cap __ro_after_init; +uintcap_t cheri_user_root_cid_cap __ro_after_init; +uintcap_t cheri_user_root_all_cap __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_user_root_cap;
- 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(void * __capability cap, size_t len, cheri_perms_t perms) +{
- ptraddr_t addr = untagged_addr(cheri_address_get(cap));
- /*
* The base address (as returned by cheri_base_get()) is never tagged,* that is its top byte is always canonical, so no need for* untagged_addr().*/- ptraddr_t base = cheri_base_get(cap);
- 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.38.1
linux-morello mailing list -- linux-morello@op-lists.linaro.org To unsubscribe send an email to linux-morello-leave@op-lists.linaro.org
On 31/01/2023 12:24, Beata Michalska wrote:
diff --git a/include/linux/cheri.h b/include/linux/cheri.h new file mode 100644 index 000000000000..5f95f1ac8f01 --- /dev/null +++ b/include/linux/cheri.h @@ -0,0 +1,132 @@ +/* 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_HAVE_ARCH_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
+/**
- cheri_build_user_cap() - Create a userspace capability.
- @addr: Requested capability address.
- @len: Requested capability length.
- @perms: Requested capability permissions.
- Return: A new capability derived from cheri_user_root_cap. Its 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 regular 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);
The returned capability might be invalid as of having tag cleared. I assume in most of the cases it will be just fine, but wondering if it would make sense to return either capability with null capability , or one that could be validated with PTR_ERR somewhat ? Just thinking at loud here, how one would verify the result is valid (currently it would need check tag but maybe we could utilize more generic way of validating pointers ... On the other hand those new helpers are available only when __CHERI__ is defined so I assume there are only to be used in CHERI-related contexts ?
Yes, those helpers can only be used directly by CHERI-specific code. As to the issue of the returned capability being invalid, that would be a bug, i.e. the caller asked for bounds that are not representable (in the exact case) or are not within the userspace range. That's why there is a WARN() in the implementation: in such a case, the caller broke the contract and we report a bug. I think it makes more sense to do it this way than require the caller to check for errors, because there is no "normal error" here, only invalid usage.
+/**
- 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.
- Return: A new capability derived from cheri_user_root_cap. 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 regular 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);+/**
- cheri_check_cap() - Check whether a capability gives access to a range of
addresses.
- @cap: Capability to check.
- @len: Length of the access.
- @perms: Required permissions.
- Checks whether @cap 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.
- Return: true if @cap passes the checks.
- */
+bool cheri_check_cap(void * __capability cap, size_t len, cheri_perms_t perms);
Minor but maybe void * __capability with const qualifier ?
Makes sense yes, capabilities are still pointers after all, and const correctness never hurts :)
+/*
- Root capabilities. Should be set in arch code during the early init phase,
- read-only after that.
- cheri_user_root_cap is the standard root capability to derive new regular
- (data/code) capabilities from. It does not include the special permissions
- Seal/Unseal and CompartmentID; those are available separately via
- cheri_user_root_{seal,cid}_cap. Finally cheri_user_root_all_cap includes all
- permissions accessible to userspace and is ultimately the root of all user
- capabilities; it should only be used in very specific situations.
- The helpers above should be used instead where possible.
- */
+extern uintcap_t cheri_user_root_cap; /* Userspace (data/code) root */ +extern uintcap_t cheri_user_root_seal_cap; /* Userspace sealing root */ +extern uintcap_t cheri_user_root_cid_cap; /* Userspace compartment ID root */ +extern uintcap_t cheri_user_root_all_cap; /* Userspace root (all permissions) */
xxx_root_all_cap somehwat reads to me as 'all capabilities' instead of 'capability with all permissions' even the context guards the interpretation, but maybe xxx_root_fill_cap instead ?
Right, that's the issue with this naming scheme. I'm not sure what you're referring to with "fill" though? How about explicitly "allperms"?
Kevin
+#endif /* __CHERI__ */
+#endif /* _LINUX_CHERI_H */
On Thu, Feb 02, 2023 at 09:56:16AM +0100, Kevin Brodsky wrote:
On 31/01/2023 12:24, Beata Michalska wrote:
diff --git a/include/linux/cheri.h b/include/linux/cheri.h new file mode 100644 index 000000000000..5f95f1ac8f01 --- /dev/null +++ b/include/linux/cheri.h @@ -0,0 +1,132 @@ +/* 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_HAVE_ARCH_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
+/**
- cheri_build_user_cap() - Create a userspace capability.
- @addr: Requested capability address.
- @len: Requested capability length.
- @perms: Requested capability permissions.
- Return: A new capability derived from cheri_user_root_cap. Its 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 regular 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);
The returned capability might be invalid as of having tag cleared. I assume in most of the cases it will be just fine, but wondering if it would make sense to return either capability with null capability , or one that could be validated with PTR_ERR somewhat ? Just thinking at loud here, how one would verify the result is valid (currently it would need check tag but maybe we could utilize more generic way of validating pointers ... On the other hand those new helpers are available only when __CHERI__ is defined so I assume there are only to be used in CHERI-related contexts ?
Yes, those helpers can only be used directly by CHERI-specific code. As to the issue of the returned capability being invalid, that would be a bug, i.e. the caller asked for bounds that are not representable (in the exact case) or are not within the userspace range. That's why there is a WARN() in the implementation: in such a case, the caller broke the contract and we report a bug. I think it makes more sense to do it this way than require the caller to check for errors, because there is no "normal error" here, only invalid usage.
Noted & agreed.
+/**
- 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.
- Return: A new capability derived from cheri_user_root_cap. 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 regular 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);+/**
- cheri_check_cap() - Check whether a capability gives access to a range of
addresses.
- @cap: Capability to check.
- @len: Length of the access.
- @perms: Required permissions.
- Checks whether @cap 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.
- Return: true if @cap passes the checks.
- */
+bool cheri_check_cap(void * __capability cap, size_t len, cheri_perms_t perms);
Minor but maybe void * __capability with const qualifier ?
Makes sense yes, capabilities are still pointers after all, and const correctness never hurts :)
+/*
- Root capabilities. Should be set in arch code during the early init phase,
- read-only after that.
- cheri_user_root_cap is the standard root capability to derive new regular
- (data/code) capabilities from. It does not include the special permissions
- Seal/Unseal and CompartmentID; those are available separately via
- cheri_user_root_{seal,cid}_cap. Finally cheri_user_root_all_cap includes all
- permissions accessible to userspace and is ultimately the root of all user
- capabilities; it should only be used in very specific situations.
- The helpers above should be used instead where possible.
- */
+extern uintcap_t cheri_user_root_cap; /* Userspace (data/code) root */ +extern uintcap_t cheri_user_root_seal_cap; /* Userspace sealing root */ +extern uintcap_t cheri_user_root_cid_cap; /* Userspace compartment ID root */ +extern uintcap_t cheri_user_root_all_cap; /* Userspace root (all permissions) */
xxx_root_all_cap somehwat reads to me as 'all capabilities' instead of 'capability with all permissions' even the context guards the interpretation, but maybe xxx_root_fill_cap instead ?
Right, that's the issue with this naming scheme. I'm not sure what you're referring to with "fill" though? How about explicitly "allperms"?
My usual misspelling ... I meant 'full' as of : cheri_user_root_full_cap. Having 'allperms' seems adequate as well.
--- BR B.
Kevin
+#endif /* __CHERI__ */
+#endif /* _LINUX_CHERI_H */
On 02/02/2023 17:05, Beata Michalska wrote:
+/*
- Root capabilities. Should be set in arch code during the early init phase,
- read-only after that.
- cheri_user_root_cap is the standard root capability to derive new regular
- (data/code) capabilities from. It does not include the special permissions
- Seal/Unseal and CompartmentID; those are available separately via
- cheri_user_root_{seal,cid}_cap. Finally cheri_user_root_all_cap includes all
- permissions accessible to userspace and is ultimately the root of all user
- capabilities; it should only be used in very specific situations.
- The helpers above should be used instead where possible.
- */
+extern uintcap_t cheri_user_root_cap; /* Userspace (data/code) root */ +extern uintcap_t cheri_user_root_seal_cap; /* Userspace sealing root */ +extern uintcap_t cheri_user_root_cid_cap; /* Userspace compartment ID root */ +extern uintcap_t cheri_user_root_all_cap; /* Userspace root (all permissions) */
xxx_root_all_cap somehwat reads to me as 'all capabilities' instead of 'capability with all permissions' even the context guards the interpretation, but maybe xxx_root_fill_cap instead ?
Right, that's the issue with this naming scheme. I'm not sure what you're referring to with "fill" though? How about explicitly "allperms"?
My usual misspelling ... I meant 'full' as of : cheri_user_root_full_cap. Having 'allperms' seems adequate as well.
All right, I think I'll go for "allperms" then, a bit more explicit.
Kevin
Implement the Morello-specific aspects of linux/cheri.h, that is:
* Override standard permission sets to include the appropriate Morello-specific permissions.
* Initialise the global root capabilities appropriately.
Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com --- arch/arm64/Kconfig | 1 + arch/arm64/include/asm/cheri.h | 11 +++++++ arch/arm64/kernel/morello.c | 58 ++++++++++++++++++++++++++-------- 3 files changed, 56 insertions(+), 14 deletions(-) create mode 100644 arch/arm64/include/asm/cheri.h
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index c784d8664a40..e5a330b1e0bd 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -29,6 +29,7 @@ config ARM64 select ARCH_HAS_GCOV_PROFILE_ALL select ARCH_HAS_GIGANTIC_PAGE select ARCH_HAS_KCOV + select HAVE_ARCH_CHERI_H select ARCH_HAS_USER_PTR_H select ARCH_HAS_KEEPINITRD select ARCH_HAS_MEMBARRIER_SYNC_CORE diff --git a/arch/arm64/include/asm/cheri.h b/arch/arm64/include/asm/cheri.h new file mode 100644 index 000000000000..eaae6ed88db5 --- /dev/null +++ b/arch/arm64/include/asm/cheri.h @@ -0,0 +1,11 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +#ifndef __ASM_CHERI_H +#define __ASM_CHERI_H + +#define CHERI_PERMS_READ \ + (CHERI_PERM_LOAD | CHERI_PERM_LOAD_CAP | ARM_CAP_PERMISSION_MUTABLE_LOAD) + +#define CHERI_PERMS_EXEC \ + (CHERI_PERM_EXECUTE | CHERI_PERM_SYSTEM_REGS | ARM_CAP_PERMISSION_EXECUTIVE) + +#endif /* __ASM_CHERI_H */ diff --git a/arch/arm64/kernel/morello.c b/arch/arm64/kernel/morello.c index ccbf26e77919..0bb7b3dbedec 100644 --- a/arch/arm64/kernel/morello.c +++ b/arch/arm64/kernel/morello.c @@ -5,10 +5,9 @@
#define pr_fmt(fmt) "morello: " fmt
-#include <cheriintrin.h> - #include <linux/cache.h> #include <linux/capability.h> +#include <linux/cheri.h> #include <linux/compat.h> #include <linux/mm.h> #include <linux/printk.h> @@ -21,10 +20,6 @@ #include <asm/morello.h> #include <asm/ptrace.h>
-#ifdef CONFIG_CHERI_PURECAP_UABI -#include <cheriintrin.h> -#endif - /* Private functions implemented in morello.S */ void __morello_cap_lo_hi_tag(uintcap_t cap, u64 *lo_val, u64 *hi_val, u8 *tag); @@ -295,8 +290,8 @@ static void __init check_root_cap(uintcap_t cap) __morello_cap_lo_hi_tag(cap, &lo_val, &hi_val, &tag);
/* - * Check that DDC has the reset value, otherwise morello_root_cap and - * all capabilities derived from it (especially those exposed to + * Check that DDC has the reset value, otherwise root capabilities and + * all capabilities derived from them (notably those exposed to * userspace) may not be reliable. */ if (!(tag == 1 && @@ -307,17 +302,52 @@ static void __init check_root_cap(uintcap_t cap)
static int __init morello_cap_init(void) { -#ifdef CONFIG_CHERI_PURECAP_UABI - uintcap_t ctemp; -#endif + uintcap_t root_cap, user_root_all_cap, ctemp; + + root_cap = (uintcap_t)cheri_ddc_get(); + check_root_cap(root_cap); + + /* Initialise standard CHERI root capabilities. */ + ctemp = cheri_address_set(root_cap, 0); + /* Same upper limit as for access_ok() and __uaccess_mask_ptr() */ + ctemp = cheri_bounds_set(ctemp, TASK_SIZE_MAX); + ctemp = cheri_perms_and(ctemp, CHERI_PERMS_ROOTCAP | CHERI_PERMS_READ | + CHERI_PERMS_WRITE | CHERI_PERMS_EXEC | + ARM_CAP_PERMISSION_BRANCH_SEALED_PAIR | + CHERI_PERM_SEAL | CHERI_PERM_UNSEAL | + ARM_CAP_PERMISSION_COMPARTMENT_ID); + user_root_all_cap = ctemp; + + cheri_user_root_all_cap = user_root_all_cap; + + ctemp = user_root_all_cap; + ctemp = cheri_perms_and(ctemp, CHERI_PERMS_ROOTCAP | CHERI_PERMS_READ | + CHERI_PERMS_WRITE | CHERI_PERMS_EXEC | + ARM_CAP_PERMISSION_BRANCH_SEALED_PAIR); + cheri_user_root_cap = ctemp; + + ctemp = user_root_all_cap; + /* + * All object types, not a final decision - some of them may be later + * reserved to the kernel. + */ + ctemp = cheri_bounds_set(ctemp, 1u << 15); + ctemp = cheri_perms_and(ctemp, CHERI_PERM_GLOBAL | + CHERI_PERM_SEAL | CHERI_PERM_UNSEAL); + cheri_user_root_seal_cap = ctemp;
- morello_root_cap = (uintcap_t)cheri_ddc_get(); + /* Maximum userspace bounds for the time being. */ + ctemp = user_root_all_cap; + ctemp = cheri_perms_and(ctemp, CHERI_PERM_GLOBAL | + ARM_CAP_PERMISSION_COMPARTMENT_ID); + cheri_user_root_cid_cap = ctemp;
- check_root_cap(morello_root_cap); + /* Initialise Morello-specific root capabilities. */ + morello_root_cap = root_cap;
#ifdef CONFIG_CHERI_PURECAP_UABI /* Initialize a capability able to unseal sentry capabilities. */ - ctemp = cheri_address_set(morello_root_cap, CHERI_OTYPE_SENTRY); + ctemp = cheri_address_set(root_cap, CHERI_OTYPE_SENTRY); ctemp = cheri_bounds_set(ctemp, 1); ctemp = cheri_perms_and(ctemp, CHERI_PERM_GLOBAL | CHERI_PERM_UNSEAL); morello_sentry_unsealcap = ctemp;
Hi,
On 12/12/22 16:05, Kevin Brodsky wrote:
Implement the Morello-specific aspects of linux/cheri.h, that is:
Override standard permission sets to include the appropriate Morello-specific permissions.
Initialise the global root capabilities appropriately.
Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com
arch/arm64/Kconfig | 1 + arch/arm64/include/asm/cheri.h | 11 +++++++ arch/arm64/kernel/morello.c | 58 ++++++++++++++++++++++++++-------- 3 files changed, 56 insertions(+), 14 deletions(-) create mode 100644 arch/arm64/include/asm/cheri.h
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index c784d8664a40..e5a330b1e0bd 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -29,6 +29,7 @@ config ARM64 select ARCH_HAS_GCOV_PROFILE_ALL select ARCH_HAS_GIGANTIC_PAGE select ARCH_HAS_KCOV
- select HAVE_ARCH_CHERI_H select ARCH_HAS_USER_PTR_H select ARCH_HAS_KEEPINITRD select ARCH_HAS_MEMBARRIER_SYNC_CORE
diff --git a/arch/arm64/include/asm/cheri.h b/arch/arm64/include/asm/cheri.h new file mode 100644 index 000000000000..eaae6ed88db5 --- /dev/null +++ b/arch/arm64/include/asm/cheri.h @@ -0,0 +1,11 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +#ifndef __ASM_CHERI_H +#define __ASM_CHERI_H
+#define CHERI_PERMS_READ \
- (CHERI_PERM_LOAD | CHERI_PERM_LOAD_CAP | ARM_CAP_PERMISSION_MUTABLE_LOAD)
+#define CHERI_PERMS_EXEC \
- (CHERI_PERM_EXECUTE | CHERI_PERM_SYSTEM_REGS | ARM_CAP_PERMISSION_EXECUTIVE)
+#endif /* __ASM_CHERI_H */ diff --git a/arch/arm64/kernel/morello.c b/arch/arm64/kernel/morello.c index ccbf26e77919..0bb7b3dbedec 100644 --- a/arch/arm64/kernel/morello.c +++ b/arch/arm64/kernel/morello.c @@ -5,10 +5,9 @@ #define pr_fmt(fmt) "morello: " fmt -#include <cheriintrin.h>
- #include <linux/cache.h> #include <linux/capability.h>
+#include <linux/cheri.h> #include <linux/compat.h> #include <linux/mm.h> #include <linux/printk.h> @@ -21,10 +20,6 @@ #include <asm/morello.h> #include <asm/ptrace.h> -#ifdef CONFIG_CHERI_PURECAP_UABI -#include <cheriintrin.h> -#endif
- /* Private functions implemented in morello.S */ void __morello_cap_lo_hi_tag(uintcap_t cap, u64 *lo_val, u64 *hi_val, u8 *tag);
@@ -295,8 +290,8 @@ static void __init check_root_cap(uintcap_t cap) __morello_cap_lo_hi_tag(cap, &lo_val, &hi_val, &tag); /*
* Check that DDC has the reset value, otherwise morello_root_cap and* all capabilities derived from it (especially those exposed to
* Check that DDC has the reset value, otherwise root capabilities and* all capabilities derived from them (notably those exposed to*/ if (!(tag == 1 &&
- userspace) may not be reliable.
@@ -307,17 +302,52 @@ static void __init check_root_cap(uintcap_t cap) static int __init morello_cap_init(void) { -#ifdef CONFIG_CHERI_PURECAP_UABI
- uintcap_t ctemp;
-#endif
- uintcap_t root_cap, user_root_all_cap, ctemp;
- root_cap = (uintcap_t)cheri_ddc_get();
- check_root_cap(root_cap);
- /* Initialise standard CHERI root capabilities. */
- ctemp = cheri_address_set(root_cap, 0);
- /* Same upper limit as for access_ok() and __uaccess_mask_ptr() */
- ctemp = cheri_bounds_set(ctemp, TASK_SIZE_MAX);
- ctemp = cheri_perms_and(ctemp, CHERI_PERMS_ROOTCAP | CHERI_PERMS_READ |
CHERI_PERMS_WRITE | CHERI_PERMS_EXEC |ARM_CAP_PERMISSION_BRANCH_SEALED_PAIR |CHERI_PERM_SEAL | CHERI_PERM_UNSEAL |ARM_CAP_PERMISSION_COMPARTMENT_ID);- user_root_all_cap = ctemp;
- cheri_user_root_all_cap = user_root_all_cap;
Nit: This local variable user_root_all_cap seems to be extra. cheri_user_root_all_cap can be used to derive capabilities.
- ctemp = user_root_all_cap;
- ctemp = cheri_perms_and(ctemp, CHERI_PERMS_ROOTCAP | CHERI_PERMS_READ |
CHERI_PERMS_WRITE | CHERI_PERMS_EXEC |ARM_CAP_PERMISSION_BRANCH_SEALED_PAIR);- cheri_user_root_cap = ctemp;
- ctemp = user_root_all_cap;
- /*
* All object types, not a final decision - some of them may be later* reserved to the kernel.*/- ctemp = cheri_bounds_set(ctemp, 1u << 15);
Nit: May a macro like MAX_OBJECT_TYPE for (1u << 15) will make it clear. Although a description for it is there.
- ctemp = cheri_perms_and(ctemp, CHERI_PERM_GLOBAL |
CHERI_PERM_SEAL | CHERI_PERM_UNSEAL);- cheri_user_root_seal_cap = ctemp;
- morello_root_cap = (uintcap_t)cheri_ddc_get();
- /* Maximum userspace bounds for the time being. */
- ctemp = user_root_all_cap;
- ctemp = cheri_perms_and(ctemp, CHERI_PERM_GLOBAL |
ARM_CAP_PERMISSION_COMPARTMENT_ID);- cheri_user_root_cid_cap = ctemp;
- check_root_cap(morello_root_cap);
- /* Initialise Morello-specific root capabilities. */
- morello_root_cap = root_cap;
#ifdef CONFIG_CHERI_PURECAP_UABI /* Initialize a capability able to unseal sentry capabilities. */
- ctemp = cheri_address_set(morello_root_cap, CHERI_OTYPE_SENTRY);
- ctemp = cheri_address_set(root_cap, CHERI_OTYPE_SENTRY); ctemp = cheri_bounds_set(ctemp, 1); ctemp = cheri_perms_and(ctemp, CHERI_PERM_GLOBAL | CHERI_PERM_UNSEAL); morello_sentry_unsealcap = ctemp;
On 18/01/2023 09:22, Amit Daniel Kachhap wrote:
@@ -307,17 +302,52 @@ static void __init check_root_cap(uintcap_t cap) Â Â static int __init morello_cap_init(void) Â { -#ifdef CONFIG_CHERI_PURECAP_UABI -Â Â Â uintcap_t ctemp; -#endif +Â Â Â uintcap_t root_cap, user_root_all_cap, ctemp;
+Â Â Â root_cap = (uintcap_t)cheri_ddc_get(); +Â Â Â check_root_cap(root_cap);
+Â Â Â /* Initialise standard CHERI root capabilities. */ +Â Â Â ctemp = cheri_address_set(root_cap, 0); +Â Â Â /* Same upper limit as for access_ok() and __uaccess_mask_ptr() */ +Â Â Â ctemp = cheri_bounds_set(ctemp, TASK_SIZE_MAX); +Â Â Â ctemp = cheri_perms_and(ctemp, CHERI_PERMS_ROOTCAP | CHERI_PERMS_READ | +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â CHERI_PERMS_WRITE | CHERI_PERMS_EXEC | +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ARM_CAP_PERMISSION_BRANCH_SEALED_PAIR | +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â CHERI_PERM_SEAL | CHERI_PERM_UNSEAL | +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ARM_CAP_PERMISSION_COMPARTMENT_ID); +Â Â Â user_root_all_cap = ctemp;
+Â Â Â cheri_user_root_all_cap = user_root_all_cap;
Nit: This local variable user_root_all_cap seems to be extra. cheri_user_root_all_cap can be used to derive capabilities.
Right so I introduced this local variable as I thought it was better not to keep reading cheri_user_root_all_cap as it's a global. However a quick experiment on Compiler Explorer shows that I underestimated Clang and it is perfectly happy to assume that there is no need to reload the global every time, meaning that the generated code is exactly the same without using the local variable.
Since I don't really like the way the code looks with that additional variable either, very happy to remove it. Thanks for the nudge :)
+Â Â Â ctemp = user_root_all_cap; +Â Â Â ctemp = cheri_perms_and(ctemp, CHERI_PERMS_ROOTCAP | CHERI_PERMS_READ | +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â CHERI_PERMS_WRITE | CHERI_PERMS_EXEC | +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ARM_CAP_PERMISSION_BRANCH_SEALED_PAIR); +Â Â Â cheri_user_root_cap = ctemp;
+Â Â Â ctemp = user_root_all_cap; +Â Â Â /* +Â Â Â Â * All object types, not a final decision - some of them may be later +Â Â Â Â * reserved to the kernel. +Â Â Â Â */ +Â Â Â ctemp = cheri_bounds_set(ctemp, 1u << 15);
Nit: May a macro like MAX_OBJECT_TYPE for (1u << 15) will make it clear.
Sure, it's Morello-specific so I'll add it directly in this file. It's not actually the maximum object type (often creates confusion as that's 2^15 - 1), so I would introduce CAP_OTYPE_FIELD_BITS = 15 instead.
Kevin
Although a description for it is there.
+Â Â Â ctemp = cheri_perms_and(ctemp, CHERI_PERM_GLOBAL | +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â CHERI_PERM_SEAL | CHERI_PERM_UNSEAL); +Â Â Â cheri_user_root_seal_cap = ctemp; Â -Â Â Â morello_root_cap = (uintcap_t)cheri_ddc_get(); +Â Â Â /* Maximum userspace bounds for the time being. */ +Â Â Â ctemp = user_root_all_cap; +Â Â Â ctemp = cheri_perms_and(ctemp, CHERI_PERM_GLOBAL | +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ARM_CAP_PERMISSION_COMPARTMENT_ID); +Â Â Â cheri_user_root_cid_cap = ctemp; Â -Â Â Â check_root_cap(morello_root_cap); +Â Â Â /* Initialise Morello-specific root capabilities. */ +Â Â Â morello_root_cap = root_cap; Â Â #ifdef CONFIG_CHERI_PURECAP_UABI Â Â Â Â Â /* Initialize a capability able to unseal sentry capabilities. */ -Â Â Â ctemp = cheri_address_set(morello_root_cap, CHERI_OTYPE_SENTRY); +Â Â Â ctemp = cheri_address_set(root_cap, CHERI_OTYPE_SENTRY); Â Â Â Â Â ctemp = cheri_bounds_set(ctemp, 1); Â Â Â Â Â ctemp = cheri_perms_and(ctemp, CHERI_PERM_GLOBAL | CHERI_PERM_UNSEAL); Â Â Â Â Â morello_sentry_unsealcap = ctemp;
On Mon, Dec 12, 2022 at 10:35:41AM +0000, Kevin Brodsky wrote:
Implement the Morello-specific aspects of linux/cheri.h, that is:
Override standard permission sets to include the appropriate Morello-specific permissions.
Initialise the global root capabilities appropriately.
Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com
arch/arm64/Kconfig | 1 + arch/arm64/include/asm/cheri.h | 11 +++++++ arch/arm64/kernel/morello.c | 58 ++++++++++++++++++++++++++-------- 3 files changed, 56 insertions(+), 14 deletions(-) create mode 100644 arch/arm64/include/asm/cheri.h
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index c784d8664a40..e5a330b1e0bd 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -29,6 +29,7 @@ config ARM64 select ARCH_HAS_GCOV_PROFILE_ALL select ARCH_HAS_GIGANTIC_PAGE select ARCH_HAS_KCOV
- select HAVE_ARCH_CHERI_H select ARCH_HAS_USER_PTR_H select ARCH_HAS_KEEPINITRD select ARCH_HAS_MEMBARRIER_SYNC_CORE
diff --git a/arch/arm64/include/asm/cheri.h b/arch/arm64/include/asm/cheri.h new file mode 100644 index 000000000000..eaae6ed88db5 --- /dev/null +++ b/arch/arm64/include/asm/cheri.h @@ -0,0 +1,11 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +#ifndef __ASM_CHERI_H +#define __ASM_CHERI_H
+#define CHERI_PERMS_READ \
- (CHERI_PERM_LOAD | CHERI_PERM_LOAD_CAP | ARM_CAP_PERMISSION_MUTABLE_LOAD)
+#define CHERI_PERMS_EXEC \
- (CHERI_PERM_EXECUTE | CHERI_PERM_SYSTEM_REGS | ARM_CAP_PERMISSION_EXECUTIVE)
+#endif /* __ASM_CHERI_H */ diff --git a/arch/arm64/kernel/morello.c b/arch/arm64/kernel/morello.c index ccbf26e77919..0bb7b3dbedec 100644 --- a/arch/arm64/kernel/morello.c +++ b/arch/arm64/kernel/morello.c @@ -5,10 +5,9 @@ #define pr_fmt(fmt) "morello: " fmt -#include <cheriintrin.h>
#include <linux/cache.h> #include <linux/capability.h> +#include <linux/cheri.h> #include <linux/compat.h> #include <linux/mm.h> #include <linux/printk.h> @@ -21,10 +20,6 @@ #include <asm/morello.h> #include <asm/ptrace.h> -#ifdef CONFIG_CHERI_PURECAP_UABI -#include <cheriintrin.h> -#endif
/* Private functions implemented in morello.S */ void __morello_cap_lo_hi_tag(uintcap_t cap, u64 *lo_val, u64 *hi_val, u8 *tag); @@ -295,8 +290,8 @@ static void __init check_root_cap(uintcap_t cap) __morello_cap_lo_hi_tag(cap, &lo_val, &hi_val, &tag); /*
* Check that DDC has the reset value, otherwise morello_root_cap and* all capabilities derived from it (especially those exposed to
* Check that DDC has the reset value, otherwise root capabilities and* all capabilities derived from them (notably those exposed to*/ if (!(tag == 1 &&
- userspace) may not be reliable.
@@ -307,17 +302,52 @@ static void __init check_root_cap(uintcap_t cap) static int __init morello_cap_init(void) { -#ifdef CONFIG_CHERI_PURECAP_UABI
- uintcap_t ctemp;
-#endif
- uintcap_t root_cap, user_root_all_cap, ctemp;
- root_cap = (uintcap_t)cheri_ddc_get();
- check_root_cap(root_cap);
- /* Initialise standard CHERI root capabilities. */
- ctemp = cheri_address_set(root_cap, 0);
- /* Same upper limit as for access_ok() and __uaccess_mask_ptr() */
- ctemp = cheri_bounds_set(ctemp, TASK_SIZE_MAX);
Out of curiosity : why not *_set_exact ?
- ctemp = cheri_perms_and(ctemp, CHERI_PERMS_ROOTCAP | CHERI_PERMS_READ |
CHERI_PERMS_WRITE | CHERI_PERMS_EXEC |ARM_CAP_PERMISSION_BRANCH_SEALED_PAIR |CHERI_PERM_SEAL | CHERI_PERM_UNSEAL |ARM_CAP_PERMISSION_COMPARTMENT_ID);- user_root_all_cap = ctemp;
- cheri_user_root_all_cap = user_root_all_cap;
- ctemp = user_root_all_cap;
I think this one is spurious (?): ctemp has not changed since the assignment two lines above.
- ctemp = cheri_perms_and(ctemp, CHERI_PERMS_ROOTCAP | CHERI_PERMS_READ |
CHERI_PERMS_WRITE | CHERI_PERMS_EXEC |ARM_CAP_PERMISSION_BRANCH_SEALED_PAIR);- cheri_user_root_cap = ctemp;
This will probably be optimized out but it could be combined with the cheri_perms_and above and you could drop the following two assignments. Similar optimization could also be done below.
--- BR B.
- ctemp = user_root_all_cap;
- /*
* All object types, not a final decision - some of them may be later* reserved to the kernel.*/- ctemp = cheri_bounds_set(ctemp, 1u << 15);
- ctemp = cheri_perms_and(ctemp, CHERI_PERM_GLOBAL |
CHERI_PERM_SEAL | CHERI_PERM_UNSEAL);- cheri_user_root_seal_cap = ctemp;
- morello_root_cap = (uintcap_t)cheri_ddc_get();
- /* Maximum userspace bounds for the time being. */
- ctemp = user_root_all_cap;
- ctemp = cheri_perms_and(ctemp, CHERI_PERM_GLOBAL |
ARM_CAP_PERMISSION_COMPARTMENT_ID);- cheri_user_root_cid_cap = ctemp;
- check_root_cap(morello_root_cap);
- /* Initialise Morello-specific root capabilities. */
- morello_root_cap = root_cap;
#ifdef CONFIG_CHERI_PURECAP_UABI /* Initialize a capability able to unseal sentry capabilities. */
- ctemp = cheri_address_set(morello_root_cap, CHERI_OTYPE_SENTRY);
- ctemp = cheri_address_set(root_cap, CHERI_OTYPE_SENTRY); ctemp = cheri_bounds_set(ctemp, 1); ctemp = cheri_perms_and(ctemp, CHERI_PERM_GLOBAL | CHERI_PERM_UNSEAL); morello_sentry_unsealcap = ctemp;
-- 2.38.1
linux-morello mailing list -- linux-morello@op-lists.linaro.org To unsubscribe send an email to linux-morello-leave@op-lists.linaro.org
On 31/01/2023 12:25, Beata Michalska wrote:
diff --git a/arch/arm64/kernel/morello.c b/arch/arm64/kernel/morello.c index ccbf26e77919..0bb7b3dbedec 100644 --- a/arch/arm64/kernel/morello.c +++ b/arch/arm64/kernel/morello.c @@ -5,10 +5,9 @@ #define pr_fmt(fmt) "morello: " fmt -#include <cheriintrin.h>
#include <linux/cache.h> #include <linux/capability.h> +#include <linux/cheri.h> #include <linux/compat.h> #include <linux/mm.h> #include <linux/printk.h> @@ -21,10 +20,6 @@ #include <asm/morello.h> #include <asm/ptrace.h> -#ifdef CONFIG_CHERI_PURECAP_UABI -#include <cheriintrin.h> -#endif
/* Private functions implemented in morello.S */ void __morello_cap_lo_hi_tag(uintcap_t cap, u64 *lo_val, u64 *hi_val, u8 *tag); @@ -295,8 +290,8 @@ static void __init check_root_cap(uintcap_t cap) __morello_cap_lo_hi_tag(cap, &lo_val, &hi_val, &tag); /*
* Check that DDC has the reset value, otherwise morello_root_cap and* all capabilities derived from it (especially those exposed to
* Check that DDC has the reset value, otherwise root capabilities and* all capabilities derived from them (notably those exposed to*/ if (!(tag == 1 &&
- userspace) may not be reliable.
@@ -307,17 +302,52 @@ static void __init check_root_cap(uintcap_t cap) static int __init morello_cap_init(void) { -#ifdef CONFIG_CHERI_PURECAP_UABI
- uintcap_t ctemp;
-#endif
- uintcap_t root_cap, user_root_all_cap, ctemp;
- root_cap = (uintcap_t)cheri_ddc_get();
- check_root_cap(root_cap);
- /* Initialise standard CHERI root capabilities. */
- ctemp = cheri_address_set(root_cap, 0);
- /* Same upper limit as for access_ok() and __uaccess_mask_ptr() */
- ctemp = cheri_bounds_set(ctemp, TASK_SIZE_MAX);
Out of curiosity : why not *_set_exact ?
Because in principle, that would require checking that the range is representable. If somehow it wasn't we would have no choice but to expand it to make it representable, which is exactly the same as using the inexact variant. OTOH if we assume that the range is indeed representable, then using one or the other makes no difference.
- ctemp = cheri_perms_and(ctemp, CHERI_PERMS_ROOTCAP | CHERI_PERMS_READ |
CHERI_PERMS_WRITE | CHERI_PERMS_EXEC |ARM_CAP_PERMISSION_BRANCH_SEALED_PAIR |CHERI_PERM_SEAL | CHERI_PERM_UNSEAL |ARM_CAP_PERMISSION_COMPARTMENT_ID);- user_root_all_cap = ctemp;
- cheri_user_root_all_cap = user_root_all_cap;
- ctemp = user_root_all_cap;
I think this one is spurious (?): ctemp has not changed since the assignment two lines above.
See Amit's comment and my reply, I thought I'd introduce a local variable to avoid continuously reading a global, but in fact Clang is very happy to optimise that out too, so I've got rid of user_root_all_cap altogether.
- ctemp = cheri_perms_and(ctemp, CHERI_PERMS_ROOTCAP | CHERI_PERMS_READ |
CHERI_PERMS_WRITE | CHERI_PERMS_EXEC |ARM_CAP_PERMISSION_BRANCH_SEALED_PAIR);- cheri_user_root_cap = ctemp;
This will probably be optimized out but it could be combined with the cheri_perms_and above and you could drop the following two assignments. Similar optimization could also be done below.
This is certainly optimised out, such local variables are only a matter of style. I've thought about a number of options but in the end it feels like using a temporary for all operations is more consistent, though it does look a bit weird when there is only one operation (i.e. setting permissions). Given the length of the globals' names, I'm not sure doing it in one statement is clearer though.
Thinking some more, we could get a little fancy and introduce some helper to set permissions and bounds. Since we don't always need to set the bounds, that argument should be optional. Conveniently we've found a good macro trick to do that in LTP, so we could reuse it here, something like:
static uintcap_t __build_cap(uintcap_t root, cheri_perms_t perms, size_t length) { Â Â Â uintcap_t c = root;
c = cheri_perms_and(c, perms); Â Â Â if (length) Â Â Â Â Â Â c = cheri_bounds_set(c, length);
return c; } #define build_cap(root, perms, ...) __build_cap(root, perms, ##__VA_ARGS__, 0)
Then to keep it readable, I would use a temporary for the permissions, something like this:
cheri_perms_t perms; Â Â Â perms = CHERI_PERMS_ROOTCAP | CHERI_PERMS_READ | Â Â Â Â Â Â CHERI_PERMS_WRITE | CHERI_PERMS_EXEC | Â Â Â Â Â Â ARM_CAP_PERMISSION_BRANCH_SEALED_PAIR; Â Â Â cheri_user_root_cap = build_cap(cheri_user_root_all_cap, perms);
How does that sound?
Kevin
BR B.
- ctemp = user_root_all_cap;
- /*
* All object types, not a final decision - some of them may be later* reserved to the kernel.*/- ctemp = cheri_bounds_set(ctemp, 1u << 15);
- ctemp = cheri_perms_and(ctemp, CHERI_PERM_GLOBAL |
CHERI_PERM_SEAL | CHERI_PERM_UNSEAL);- cheri_user_root_seal_cap = ctemp;
- morello_root_cap = (uintcap_t)cheri_ddc_get();
- /* Maximum userspace bounds for the time being. */
- ctemp = user_root_all_cap;
- ctemp = cheri_perms_and(ctemp, CHERI_PERM_GLOBAL |
ARM_CAP_PERMISSION_COMPARTMENT_ID);- cheri_user_root_cid_cap = ctemp;
- check_root_cap(morello_root_cap);
- /* Initialise Morello-specific root capabilities. */
- morello_root_cap = root_cap;
#ifdef CONFIG_CHERI_PURECAP_UABI /* Initialize a capability able to unseal sentry capabilities. */
- ctemp = cheri_address_set(morello_root_cap, CHERI_OTYPE_SENTRY);
- ctemp = cheri_address_set(root_cap, CHERI_OTYPE_SENTRY); ctemp = cheri_bounds_set(ctemp, 1); ctemp = cheri_perms_and(ctemp, CHERI_PERM_GLOBAL | CHERI_PERM_UNSEAL); morello_sentry_unsealcap = ctemp;
On Thu, Feb 02, 2023 at 11:20:56AM +0100, Kevin Brodsky wrote:
On 31/01/2023 12:25, Beata Michalska wrote:
diff --git a/arch/arm64/kernel/morello.c b/arch/arm64/kernel/morello.c index ccbf26e77919..0bb7b3dbedec 100644 --- a/arch/arm64/kernel/morello.c +++ b/arch/arm64/kernel/morello.c @@ -5,10 +5,9 @@ #define pr_fmt(fmt) "morello: " fmt -#include <cheriintrin.h>
#include <linux/cache.h> #include <linux/capability.h> +#include <linux/cheri.h> #include <linux/compat.h> #include <linux/mm.h> #include <linux/printk.h> @@ -21,10 +20,6 @@ #include <asm/morello.h> #include <asm/ptrace.h> -#ifdef CONFIG_CHERI_PURECAP_UABI -#include <cheriintrin.h> -#endif
/* Private functions implemented in morello.S */ void __morello_cap_lo_hi_tag(uintcap_t cap, u64 *lo_val, u64 *hi_val, u8 *tag); @@ -295,8 +290,8 @@ static void __init check_root_cap(uintcap_t cap) __morello_cap_lo_hi_tag(cap, &lo_val, &hi_val, &tag); /*
* Check that DDC has the reset value, otherwise morello_root_cap and* all capabilities derived from it (especially those exposed to
* Check that DDC has the reset value, otherwise root capabilities and* all capabilities derived from them (notably those exposed to*/ if (!(tag == 1 &&
- userspace) may not be reliable.
@@ -307,17 +302,52 @@ static void __init check_root_cap(uintcap_t cap) static int __init morello_cap_init(void) { -#ifdef CONFIG_CHERI_PURECAP_UABI
- uintcap_t ctemp;
-#endif
- uintcap_t root_cap, user_root_all_cap, ctemp;
- root_cap = (uintcap_t)cheri_ddc_get();
- check_root_cap(root_cap);
- /* Initialise standard CHERI root capabilities. */
- ctemp = cheri_address_set(root_cap, 0);
- /* Same upper limit as for access_ok() and __uaccess_mask_ptr() */
- ctemp = cheri_bounds_set(ctemp, TASK_SIZE_MAX);
Out of curiosity : why not *_set_exact ?
Because in principle, that would require checking that the range is representable. If somehow it wasn't we would have no choice but to expand it to make it representable, which is exactly the same as using the inexact variant. OTOH if we assume that the range is indeed representable, then using one or the other makes no difference.
Thanks for clarifying.
- ctemp = cheri_perms_and(ctemp, CHERI_PERMS_ROOTCAP | CHERI_PERMS_READ |
CHERI_PERMS_WRITE | CHERI_PERMS_EXEC |ARM_CAP_PERMISSION_BRANCH_SEALED_PAIR |CHERI_PERM_SEAL | CHERI_PERM_UNSEAL |ARM_CAP_PERMISSION_COMPARTMENT_ID);- user_root_all_cap = ctemp;
- cheri_user_root_all_cap = user_root_all_cap;
- ctemp = user_root_all_cap;
I think this one is spurious (?): ctemp has not changed since the assignment two lines above.
See Amit's comment and my reply, I thought I'd introduce a local variable to avoid continuously reading a global, but in fact Clang is very happy to optimise that out too, so I've got rid of user_root_all_cap altogether.
- ctemp = cheri_perms_and(ctemp, CHERI_PERMS_ROOTCAP | CHERI_PERMS_READ |
CHERI_PERMS_WRITE | CHERI_PERMS_EXEC |ARM_CAP_PERMISSION_BRANCH_SEALED_PAIR);- cheri_user_root_cap = ctemp;
This will probably be optimized out but it could be combined with the cheri_perms_and above and you could drop the following two assignments. Similar optimization could also be done below.
This is certainly optimised out, such local variables are only a matter of style. I've thought about a number of options but in the end it feels like using a temporary for all operations is more consistent, though it does look a bit weird when there is only one operation (i.e. setting permissions). Given the length of the globals' names, I'm not sure doing it in one statement is clearer though.
Thinking some more, we could get a little fancy and introduce some helper to set permissions and bounds. Since we don't always need to set the bounds, that argument should be optional. Conveniently we've found a good macro trick to do that in LTP, so we could reuse it here, something like:
static uintcap_t __build_cap(uintcap_t root, cheri_perms_t perms, size_t length) { Â Â Â uintcap_t c = root;
c = cheri_perms_and(c, perms); Â Â Â if (length) Â Â Â Â Â Â c = cheri_bounds_set(c, length);
return c; } #define build_cap(root, perms, ...) __build_cap(root, perms, ##__VA_ARGS__, 0)
Then to keep it readable, I would use a temporary for the permissions, something like this:
cheri_perms_t perms; Â Â Â perms = CHERI_PERMS_ROOTCAP | CHERI_PERMS_READ | Â Â Â Â Â Â CHERI_PERMS_WRITE | CHERI_PERMS_EXEC | Â Â Â Â Â Â ARM_CAP_PERMISSION_BRANCH_SEALED_PAIR; Â Â Â cheri_user_root_cap = build_cap(cheri_user_root_all_cap, perms);
How does that sound?
Sounds great actually.
--- BR B.
Kevin
BR B.
- ctemp = user_root_all_cap;
- /*
* All object types, not a final decision - some of them may be later* reserved to the kernel.*/- ctemp = cheri_bounds_set(ctemp, 1u << 15);
- ctemp = cheri_perms_and(ctemp, CHERI_PERM_GLOBAL |
CHERI_PERM_SEAL | CHERI_PERM_UNSEAL);- cheri_user_root_seal_cap = ctemp;
- morello_root_cap = (uintcap_t)cheri_ddc_get();
- /* Maximum userspace bounds for the time being. */
- ctemp = user_root_all_cap;
- ctemp = cheri_perms_and(ctemp, CHERI_PERM_GLOBAL |
ARM_CAP_PERMISSION_COMPARTMENT_ID);- cheri_user_root_cid_cap = ctemp;
- check_root_cap(morello_root_cap);
- /* Initialise Morello-specific root capabilities. */
- morello_root_cap = root_cap;
#ifdef CONFIG_CHERI_PURECAP_UABI /* Initialize a capability able to unseal sentry capabilities. */
- ctemp = cheri_address_set(morello_root_cap, CHERI_OTYPE_SENTRY);
- ctemp = cheri_address_set(root_cap, CHERI_OTYPE_SENTRY); ctemp = cheri_bounds_set(ctemp, 1); ctemp = cheri_perms_and(ctemp, CHERI_PERM_GLOBAL | CHERI_PERM_UNSEAL); morello_sentry_unsealcap = ctemp;
On 02/02/2023 17:08, Beata Michalska wrote:
- ctemp = cheri_perms_and(ctemp, CHERI_PERMS_ROOTCAP | CHERI_PERMS_READ |
CHERI_PERMS_WRITE | CHERI_PERMS_EXEC |ARM_CAP_PERMISSION_BRANCH_SEALED_PAIR);- cheri_user_root_cap = ctemp;
This will probably be optimized out but it could be combined with the cheri_perms_and above and you could drop the following two assignments. Similar optimization could also be done below.
This is certainly optimised out, such local variables are only a matter of style. I've thought about a number of options but in the end it feels like using a temporary for all operations is more consistent, though it does look a bit weird when there is only one operation (i.e. setting permissions). Given the length of the globals' names, I'm not sure doing it in one statement is clearer though.
Thinking some more, we could get a little fancy and introduce some helper to set permissions and bounds. Since we don't always need to set the bounds, that argument should be optional. Conveniently we've found a good macro trick to do that in LTP, so we could reuse it here, something like:
static uintcap_t __build_cap(uintcap_t root, cheri_perms_t perms, size_t length) { �� �uintcap_t c = root;
�� �c = cheri_perms_and(c, perms); �� �if (length) �� ���� c = cheri_bounds_set(c, length);
�� �return c; } #define build_cap(root, perms, ...) __build_cap(root, perms, ##__VA_ARGS__, 0)
Then to keep it readable, I would use a temporary for the permissions, something like this:
��� cheri_perms_t perms; �� �perms = CHERI_PERMS_ROOTCAP | CHERI_PERMS_READ | �� ���� CHERI_PERMS_WRITE | CHERI_PERMS_EXEC | �� ���� ARM_CAP_PERMISSION_BRANCH_SEALED_PAIR; �� �cheri_user_root_cap = build_cap(cheri_user_root_all_cap, perms);
How does that sound?
Sounds great actually.
Cool, will go for something like that in v3 then.
Kevin
uaddr_to_user_ptr_safe() (called by elf_uaddr_to_user_ptr()) will soon return capabilities without the special permissions Seal/Unseal/CompartmentID. We should therefore initialise AT_CHERI_{SEAL,CID}_CAP with the corresponding root capabilities that linux/cheri.h now provides.
Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com --- fs/binfmt_elf.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c index 7e391025ffe8..cea2587433db 100644 --- a/fs/binfmt_elf.c +++ b/fs/binfmt_elf.c @@ -46,13 +46,10 @@ #include <linux/cred.h> #include <linux/dax.h> #include <linux/uaccess.h> +#include <linux/cheri.h> #include <asm/param.h> #include <asm/page.h>
-#ifdef CONFIG_CHERI_PURECAP_UABI -#include <cheriintrin.h> -#endif - #ifndef ELF_COMPAT #define ELF_COMPAT 0 #endif @@ -328,9 +325,8 @@ create_elf_tables(struct linux_binprm *bprm, const struct elfhdr *exec, elf_uaddr_to_user_ptr(interp_load_info->start_elf_rx) : NULL)); NEW_AUX_ENT(AT_CHERI_STACK_CAP, elf_uaddr_to_user_ptr(0)); - NEW_AUX_ENT(AT_CHERI_SEAL_CAP, - cheri_bounds_set_exact(elf_uaddr_to_user_ptr(0), 1 << 15)); - NEW_AUX_ENT(AT_CHERI_CID_CAP, elf_uaddr_to_user_ptr(0)); + NEW_AUX_ENT(AT_CHERI_SEAL_CAP, cheri_user_root_seal_cap); + NEW_AUX_ENT(AT_CHERI_CID_CAP, cheri_user_root_cid_cap);
/* * Since the auxv entries are inserted into the mm struct before the
All capabilities in hybrid (here compat64) should now be derived from cheri_user_root_all_cap. There is no helper to derive capabilities from this root, so do it manually. This is specific to PCuABI, but the generic implementation of compat_ptr() is appropriate in !PCuABI, so we can simply #ifdef the whole definition.
Also update the comment as this does not pertain to PCuABI as such.
Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com --- arch/arm64/include/asm/compat.h | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/include/asm/compat.h b/arch/arm64/include/asm/compat.h index 9c6112ae942b..1e4e8506a8b3 100644 --- a/arch/arm64/include/asm/compat.h +++ b/arch/arm64/include/asm/compat.h @@ -18,6 +18,7 @@ typedef u16 compat_mode_t; #include <linux/types.h> #include <linux/sched.h> #include <linux/sched/task_stack.h> +#include <linux/cheri.h>
#define COMPAT_USER_HZ 100 #ifdef __AARCH64EB__ @@ -104,15 +105,17 @@ struct compat_statfs {
#define COMPAT_OFF_T_MAX 0x7fffffff
+#ifdef CONFIG_CHERI_PURECAP_UABI static inline void __user *compat_ptr(compat_uptr_t uptr) { /* - * TODO [PCuABI] - this should be done using the current user DDC, not - * the root kernel one. + * TODO [Morello] - this should be done using the current user DDC, not + * the root user capability. */ - return uaddr_to_user_ptr_safe(uptr); + return (void __user *)cheri_address_set(cheri_user_root_all_cap, uptr); } #define compat_ptr(uptr) compat_ptr(uptr) +#endif
#define compat_user_stack_pointer() (user_stack_pointer(task_pt_regs(current))) #define COMPAT_MINSIGSTKSZ 2048
Now that linux/cheri.h provides CHERI-generic functionalities, it is possible to implement uaddr_to_user_ptr* for PCuABI in an arch-agnostic way.
With this patch, the PCuABI version is implemented out of line in lib/user_ptr.c. This is notably to avoid pulling in linux/cheri.h in linux/user_ptr.h, itself included from linux/kernel.h.
This approach supersedes asm/user_ptr.h, which is no longer used and will be subsequently removed.
Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com --- include/linux/user_ptr.h | 26 ++++++++++++++------------ lib/Makefile | 1 + lib/user_ptr.c | 26 ++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 12 deletions(-) create mode 100644 lib/user_ptr.c
diff --git a/include/linux/user_ptr.h b/include/linux/user_ptr.h index 7e0278567947..5fbc01a95297 100644 --- a/include/linux/user_ptr.h +++ b/include/linux/user_ptr.h @@ -4,10 +4,6 @@
#include <linux/typecheck.h>
-#ifdef CONFIG_ARCH_HAS_USER_PTR_H -#include <asm/user_ptr.h> -#endif - /** * as_user_ptr() - Convert an arbitrary integer value to a user pointer. * @x: The integer value to convert. @@ -28,7 +24,8 @@ /* Legacy user pointer conversion macro, new code should use as_user_ptr() */ #define u64_to_user_ptr(x) as_user_ptr_strict(u64, (x))
-#ifndef uaddr_to_user_ptr +#ifdef CONFIG_CHERI_PURECAP_UABI + /** * uaddr_to_user_ptr() - Convert a user-provided address to a user pointer. * @addr: The address to set the pointer to. @@ -42,13 +39,8 @@ * When the pure-capability uABI is targeted, uses of this function bypass the * capability model and should be minimised. */ -static inline void __user *uaddr_to_user_ptr(ptraddr_t addr) -{ - return as_user_ptr(addr); -} -#endif +void __user *uaddr_to_user_ptr(ptraddr_t addr);
-#ifndef uaddr_to_user_ptr_safe /** * uaddr_to_user_ptr_safe() - Convert a kernel-generated user address to a * user pointer. @@ -60,11 +52,21 @@ static inline void __user *uaddr_to_user_ptr(ptraddr_t addr) * 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). */ +void __user *uaddr_to_user_ptr_safe(ptraddr_t addr); + +#else /* CONFIG_CHERI_PURECAP_UABI */ + +static inline void __user *uaddr_to_user_ptr(ptraddr_t addr) +{ + return as_user_ptr(addr); +} + static inline void __user *uaddr_to_user_ptr_safe(ptraddr_t addr) { return as_user_ptr(addr); } -#endif + +#endif /* CONFIG_CHERI_PURECAP_UABI */
/** * user_ptr_addr() - Extract the address of a user pointer. diff --git a/lib/Makefile b/lib/Makefile index 8dd8c00fee31..8bdd488e7e59 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -264,6 +264,7 @@ obj-$(CONFIG_STMP_DEVICE) += stmp_device.o obj-$(CONFIG_IRQ_POLL) += irq_poll.o
obj-y += cheri.o +obj-$(CONFIG_CHERI_PURECAP_UABI) += user_ptr.o
# stackdepot.c should not be instrumented or call instrumented functions. # Prevent the compiler from calling builtins like memcmp() or bcmp() from this diff --git a/lib/user_ptr.c b/lib/user_ptr.c new file mode 100644 index 000000000000..7fd67f793122 --- /dev/null +++ b/lib/user_ptr.c @@ -0,0 +1,26 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +#include <linux/bug.h> +#include <linux/cheri.h> +#include <linux/user_ptr.h> + +void __user *uaddr_to_user_ptr(ptraddr_t addr) +{ + /* + * No warning if the result is invalid as the input address is not + * controlled by the kernel. + */ + return (void __user *)cheri_address_set(cheri_user_root_cap, addr); +} + +void __user *uaddr_to_user_ptr_safe(ptraddr_t addr) +{ + void __user *ret; + + ret = (void __user *)cheri_address_set(cheri_user_root_cap, addr); + + WARN(!cheri_tag_get(ret), + "Invalid user capability created: %#lp\n", ret); + + return ret; +} +
linux/user_ptr.h now uses generic PCuABI implementations of uaddr_to_user_ptr* and no longer includes asm/user_ptr.h. Time to remove it along with the corresponding config option.
Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com --- arch/Kconfig | 3 --- arch/arm64/Kconfig | 1 - arch/arm64/include/asm/user_ptr.h | 35 ------------------------------- 3 files changed, 39 deletions(-) delete mode 100644 arch/arm64/include/asm/user_ptr.h
diff --git a/arch/Kconfig b/arch/Kconfig index 6c3c2e19b77e..16fb8f7940f9 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -1379,9 +1379,6 @@ config HAVE_ARCH_NODE_DEV_GROUP config HAVE_ARCH_CHERI_H bool
-config ARCH_HAS_USER_PTR_H - bool - config ARCH_HAS_CHERI_CAPABILITIES bool
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index e5a330b1e0bd..8b087494482d 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -30,7 +30,6 @@ config ARM64 select ARCH_HAS_GIGANTIC_PAGE select ARCH_HAS_KCOV select HAVE_ARCH_CHERI_H - select ARCH_HAS_USER_PTR_H select ARCH_HAS_KEEPINITRD select ARCH_HAS_MEMBARRIER_SYNC_CORE select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE diff --git a/arch/arm64/include/asm/user_ptr.h b/arch/arm64/include/asm/user_ptr.h deleted file mode 100644 index 323ad0301cbd..000000000000 --- a/arch/arm64/include/asm/user_ptr.h +++ /dev/null @@ -1,35 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ -#ifndef __ASM_USER_PTR_H -#define __ASM_USER_PTR_H - -#include <asm/morello.h> - -#ifdef CONFIG_CHERI_PURECAP_UABI - -static inline void __user *uaddr_to_user_ptr(ptraddr_t addr) -{ - /* - * TODO [PCuABI] - the user root capability should be used, not the - * kernel one. - */ - uintcap_t root_cap = morello_get_root_cap(); - - return (void __user *)__builtin_cheri_address_set(root_cap, addr); -} -#define uaddr_to_user_ptr(addr) uaddr_to_user_ptr(addr) - -static inline void __user *uaddr_to_user_ptr_safe(ptraddr_t addr) -{ - /* - * TODO [PCuABI] - the user root capability should be used, not the - * kernel one. - */ - uintcap_t root_cap = morello_get_root_cap(); - - return (void __user *)__builtin_cheri_address_set(root_cap, addr); -} -#define uaddr_to_user_ptr_safe(addr) uaddr_to_user_ptr_safe(addr) - -#endif /* CONFIG_CHERI_PURECAP_UABI */ - -#endif /* __ASM_USER_PTR_H */
Now that we have introduced the generic cheri_user_root_* userspace root capabilities, we can make use of them to derive the initial user capabilities (process startup and signal delivery). Note that DDC will be taken care of separately, as it is not saved in struct pt_regs and is not initialised in morello_thread_start() either.
For purecap (PCuABI) processes, we derive the capabilities from the "regular" root capability; their bounds and permissions will later be reduced further as per the PCuABI specification.
For hybrid processes (i.e. plain AArch64 ABI + capability support), the capabilities are derived from the special root capability with all permissions, cheri_user_root_all_cap. That's because we are not attempting to constrain the capabilities provided to hybrid processes in any way, and there is no mechanism to provide special permissions (Seal/Unseal/CompartmentID) separately in hybrid.
Since the purecap/hybrid initialisation is now clearly separated, we also use that opportunity to leave CSP zero-initialised in hybrid; a valid CSP should only be provided in purecap.
To avoid some ungraceful #ifdef'ing in morello_setup_signal_return(), morello_sentry_unsealcap is now always defined, as the overhead is insignificant (a 16-byte global and a few instructions on startup).
Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com --- arch/arm64/kernel/morello.c | 71 ++++++++++++++++++------------------- 1 file changed, 34 insertions(+), 37 deletions(-)
diff --git a/arch/arm64/kernel/morello.c b/arch/arm64/kernel/morello.c index 0bb7b3dbedec..7a9113e2cf5f 100644 --- a/arch/arm64/kernel/morello.c +++ b/arch/arm64/kernel/morello.c @@ -28,9 +28,7 @@ bool __morello_cap_has_executive(uintcap_t cap);
/* Not defined as static because morello.S refers to it */ uintcap_t morello_root_cap __ro_after_init; -#ifdef CONFIG_CHERI_PURECAP_UABI static uintcap_t morello_sentry_unsealcap __ro_after_init; -#endif
/* DDC_ELx reset value (low/high 64 bits), as defined in the Morello spec */ #define DDC_RESET_VAL_LOW_64 0x0 @@ -41,14 +39,9 @@ uintcap_t morello_get_root_cap(void) return morello_root_cap; }
-static void init_pcc(struct pt_regs *regs) +static bool is_pure_task(void) { - /* - * Set PCC to the root capability. There is no need to set its value to - * pc, this will be taken care of when PC is merged into PCC during - * ret_to_user. - */ - regs->pcc = morello_root_cap; + return IS_ENABLED(CONFIG_CHERI_PURECAP_UABI) && !is_compat_task(); }
static void update_regs_c64(struct pt_regs *regs, unsigned long pc) @@ -64,21 +57,24 @@ static void update_regs_c64(struct pt_regs *regs, unsigned long pc) } }
-static void init_csp(struct pt_regs *regs) -{ - /* - * TODO [PCuABI] - Adjust the bounds/permissions properly - * This should also be somehow hooked up for stack limit changes - */ - /* The actual value for CSP will be set during ret_to_user */ - regs->csp = morello_root_cap; -} - void morello_thread_start(struct pt_regs *regs, unsigned long pc) { - init_pcc(regs); update_regs_c64(regs, pc); - init_csp(regs); + + /* + * Note: there is no need to explicitly set the address of PCC/CSP as + * PC/SP are already set to the appropriate values in regs, and X/C + * register merging automatically happens during ret_to_user. + */ + if (is_pure_task()) { + /* TODO [PCuABI] - Adjust the bounds/permissions properly */ + regs->pcc = cheri_user_root_cap; + + regs->csp = cheri_user_root_cap; + } else /* Hybrid */ { + regs->pcc = cheri_user_root_all_cap; + /* CSP is null-derived in hybrid */ + } }
#ifdef CONFIG_CHERI_PURECAP_UABI @@ -97,22 +93,25 @@ void morello_setup_signal_return(struct pt_regs *regs) * point (this means in particular that the signal handler is invoked in * Executive). */ -#ifdef CONFIG_CHERI_PURECAP_UABI - if (is_compat_task()) - init_pcc(regs); - /* Unseal if the pcc has sentry object type */ - else if (cheri_is_sentry(regs->pcc)) - regs->pcc = cheri_unseal(regs->pcc, morello_sentry_unsealcap); -#else - init_pcc(regs); -#endif update_regs_c64(regs, regs->pc);
- /* - * Also set CLR to a valid capability, to allow a C64 handler to return - * to the trampoline using `ret clr`. - */ - regs->cregs[30] = morello_root_cap; + if (is_pure_task()) { + /* Unseal if the pcc has sentry object type */ + if (cheri_is_sentry(regs->pcc)) + regs->pcc = cheri_unseal(regs->pcc, + morello_sentry_unsealcap); + + /* TODO [PCuABI] - Adjust the bounds/permissions properly */ + regs->cregs[30] = cheri_user_root_cap; + } else /* Hybrid */ { + regs->pcc = cheri_user_root_all_cap; + + /* + * Also set CLR to a valid capability, to allow a C64 handler + * to return to the trampoline using `ret clr`. + */ + regs->cregs[30] = cheri_user_root_all_cap; + } }
static char *format_cap(char *buf, size_t size, uintcap_t cap) @@ -345,13 +344,11 @@ static int __init morello_cap_init(void) /* Initialise Morello-specific root capabilities. */ morello_root_cap = root_cap;
-#ifdef CONFIG_CHERI_PURECAP_UABI /* Initialize a capability able to unseal sentry capabilities. */ ctemp = cheri_address_set(root_cap, CHERI_OTYPE_SENTRY); ctemp = cheri_bounds_set(ctemp, 1); ctemp = cheri_perms_and(ctemp, CHERI_PERM_GLOBAL | CHERI_PERM_UNSEAL); morello_sentry_unsealcap = ctemp; -#endif
return 0; }
On Mon, Dec 12, 2022 at 10:35:46AM +0000, Kevin Brodsky wrote:
Now that we have introduced the generic cheri_user_root_* userspace root capabilities, we can make use of them to derive the initial user capabilities (process startup and signal delivery). Note that DDC will be taken care of separately, as it is not saved in struct pt_regs and is not initialised in morello_thread_start() either.
For purecap (PCuABI) processes, we derive the capabilities from the "regular" root capability; their bounds and permissions will later be reduced further as per the PCuABI specification.
For hybrid processes (i.e. plain AArch64 ABI + capability support), the capabilities are derived from the special root capability with all permissions, cheri_user_root_all_cap. That's because we are not attempting to constrain the capabilities provided to hybrid processes in any way, and there is no mechanism to provide special permissions (Seal/Unseal/CompartmentID) separately in hybrid.
Since the purecap/hybrid initialisation is now clearly separated, we also use that opportunity to leave CSP zero-initialised in hybrid; a valid CSP should only be provided in purecap.
To avoid some ungraceful #ifdef'ing in morello_setup_signal_return(), morello_sentry_unsealcap is now always defined, as the overhead is insignificant (a 16-byte global and a few instructions on startup).
Minor: You can use IS_ENABLED instead of #ifdef'ing, though I do not mind the change.
--- BR B.
Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com
arch/arm64/kernel/morello.c | 71 ++++++++++++++++++------------------- 1 file changed, 34 insertions(+), 37 deletions(-)
diff --git a/arch/arm64/kernel/morello.c b/arch/arm64/kernel/morello.c index 0bb7b3dbedec..7a9113e2cf5f 100644 --- a/arch/arm64/kernel/morello.c +++ b/arch/arm64/kernel/morello.c @@ -28,9 +28,7 @@ bool __morello_cap_has_executive(uintcap_t cap); /* Not defined as static because morello.S refers to it */ uintcap_t morello_root_cap __ro_after_init; -#ifdef CONFIG_CHERI_PURECAP_UABI static uintcap_t morello_sentry_unsealcap __ro_after_init; -#endif /* DDC_ELx reset value (low/high 64 bits), as defined in the Morello spec */ #define DDC_RESET_VAL_LOW_64 0x0 @@ -41,14 +39,9 @@ uintcap_t morello_get_root_cap(void) return morello_root_cap; } -static void init_pcc(struct pt_regs *regs) +static bool is_pure_task(void) {
- /*
* Set PCC to the root capability. There is no need to set its value to* pc, this will be taken care of when PC is merged into PCC during* ret_to_user.*/- regs->pcc = morello_root_cap;
- return IS_ENABLED(CONFIG_CHERI_PURECAP_UABI) && !is_compat_task();
} static void update_regs_c64(struct pt_regs *regs, unsigned long pc) @@ -64,21 +57,24 @@ static void update_regs_c64(struct pt_regs *regs, unsigned long pc) } } -static void init_csp(struct pt_regs *regs) -{
- /*
* TODO [PCuABI] - Adjust the bounds/permissions properly* This should also be somehow hooked up for stack limit changes*/- /* The actual value for CSP will be set during ret_to_user */
- regs->csp = morello_root_cap;
-}
void morello_thread_start(struct pt_regs *regs, unsigned long pc) {
- init_pcc(regs); update_regs_c64(regs, pc);
- init_csp(regs);
- /*
* Note: there is no need to explicitly set the address of PCC/CSP as* PC/SP are already set to the appropriate values in regs, and X/C* register merging automatically happens during ret_to_user.*/- if (is_pure_task()) {
/* TODO [PCuABI] - Adjust the bounds/permissions properly */regs->pcc = cheri_user_root_cap;regs->csp = cheri_user_root_cap;- } else /* Hybrid */ {
regs->pcc = cheri_user_root_all_cap;/* CSP is null-derived in hybrid */- }
} #ifdef CONFIG_CHERI_PURECAP_UABI @@ -97,22 +93,25 @@ void morello_setup_signal_return(struct pt_regs *regs) * point (this means in particular that the signal handler is invoked in * Executive). */ -#ifdef CONFIG_CHERI_PURECAP_UABI
- if (is_compat_task())
init_pcc(regs);- /* Unseal if the pcc has sentry object type */
- else if (cheri_is_sentry(regs->pcc))
regs->pcc = cheri_unseal(regs->pcc, morello_sentry_unsealcap);-#else
- init_pcc(regs);
-#endif update_regs_c64(regs, regs->pc);
- /*
* Also set CLR to a valid capability, to allow a C64 handler to return* to the trampoline using `ret clr`.*/- regs->cregs[30] = morello_root_cap;
- if (is_pure_task()) {
/* Unseal if the pcc has sentry object type */if (cheri_is_sentry(regs->pcc))regs->pcc = cheri_unseal(regs->pcc,morello_sentry_unsealcap);/* TODO [PCuABI] - Adjust the bounds/permissions properly */regs->cregs[30] = cheri_user_root_cap;- } else /* Hybrid */ {
regs->pcc = cheri_user_root_all_cap;/** Also set CLR to a valid capability, to allow a C64 handler* to return to the trampoline using `ret clr`.*/regs->cregs[30] = cheri_user_root_all_cap;- }
} static char *format_cap(char *buf, size_t size, uintcap_t cap) @@ -345,13 +344,11 @@ static int __init morello_cap_init(void) /* Initialise Morello-specific root capabilities. */ morello_root_cap = root_cap; -#ifdef CONFIG_CHERI_PURECAP_UABI /* Initialize a capability able to unseal sentry capabilities. */ ctemp = cheri_address_set(root_cap, CHERI_OTYPE_SENTRY); ctemp = cheri_bounds_set(ctemp, 1); ctemp = cheri_perms_and(ctemp, CHERI_PERM_GLOBAL | CHERI_PERM_UNSEAL); morello_sentry_unsealcap = ctemp; -#endif return 0; } -- 2.38.1
linux-morello mailing list -- linux-morello@op-lists.linaro.org To unsubscribe send an email to linux-morello-leave@op-lists.linaro.org
On 31/01/2023 12:25, Beata Michalska wrote:
To avoid some ungraceful #ifdef'ing in morello_setup_signal_return(), morello_sentry_unsealcap is now always defined, as the overhead is insignificant (a 16-byte global and a few instructions on startup).
Minor: You can use IS_ENABLED instead of #ifdef'ing, though I do not mind the change.
I don't think so, IS_ENABLED() doesn't remove the need for the whole expression to be parsable, which requires morello_sentry_unsealcap to be at least declared.
Kevin
On Thu, Feb 02, 2023 at 11:23:41AM +0100, Kevin Brodsky wrote:
On 31/01/2023 12:25, Beata Michalska wrote:
To avoid some ungraceful #ifdef'ing in morello_setup_signal_return(), morello_sentry_unsealcap is now always defined, as the overhead is insignificant (a 16-byte global and a few instructions on startup).
Minor: You can use IS_ENABLED instead of #ifdef'ing, though I do not mind the change.
I don't think so, IS_ENABLED() doesn't remove the need for the whole expression to be parsable, which requires morello_sentry_unsealcap to be at least declared.
I was thinkin about skipping the unseal call but that one is already guarded by is_pure_task so you are right, that is not an option. My bad.
--- BR B.
Kevin
Just like PCC/CSP, we can now derive the initial user DDC from the appropriate cheri_user_root_* userspace root capability. We still provide it in purecap to be consistent with the transitional PCuABI specification, but it will eventually be set to the null capability.
Checking whether we are in compat or not in assembly would be tedious, so a C wrapper is used to do the purecap/hybrid selection. morello_thread_init_user() always targets current, so to keep things simple and use the argumentless is_pure_task() helper, we remove its task_struct * argument. This is in line with other helpers called from arch_setup_new_exec().
Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com --- arch/arm64/include/asm/morello.h | 2 +- arch/arm64/kernel/morello.c | 10 ++++++++++ arch/arm64/kernel/process.c | 2 +- arch/arm64/lib/morello.S | 8 +++----- 4 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/include/asm/morello.h b/arch/arm64/include/asm/morello.h index 7ba5a527891b..e60ad5727fca 100644 --- a/arch/arm64/include/asm/morello.h +++ b/arch/arm64/include/asm/morello.h @@ -63,7 +63,7 @@ void morello_thread_set_csp(struct pt_regs *regs, user_uintptr_t sp); void *morello_capcpy(void *dst, const void *src, size_t len);
void morello_thread_start(struct pt_regs *regs, unsigned long pc); -void morello_thread_init_user(struct task_struct *tsk); +void morello_thread_init_user(void); void morello_thread_save_user_state(struct task_struct *tsk); void morello_thread_restore_user_state(struct task_struct *tsk); void morello_task_save_user_tls(struct task_struct *tsk, user_uintptr_t *tp_ptr); diff --git a/arch/arm64/kernel/morello.c b/arch/arm64/kernel/morello.c index 7a9113e2cf5f..9dc42231df20 100644 --- a/arch/arm64/kernel/morello.c +++ b/arch/arm64/kernel/morello.c @@ -25,6 +25,7 @@ void __morello_cap_lo_hi_tag(uintcap_t cap, u64 *lo_val, u64 *hi_val, u8 *tag); void __morello_merge_c_x(uintcap_t *creg, u64 xreg); bool __morello_cap_has_executive(uintcap_t cap); +void __morello_thread_init_user(struct task_struct *tsk, uintcap_t ddc);
/* Not defined as static because morello.S refers to it */ uintcap_t morello_root_cap __ro_after_init; @@ -77,6 +78,15 @@ void morello_thread_start(struct pt_regs *regs, unsigned long pc) } }
+void morello_thread_init_user(void) +{ + /* TODO [PCuABI] - Set DDC to the null capability */ + uintcap_t ddc = is_pure_task() ? cheri_user_root_cap + : cheri_user_root_all_cap; + + __morello_thread_init_user(current, ddc); +} + #ifdef CONFIG_CHERI_PURECAP_UABI void morello_thread_set_csp(struct pt_regs *regs, user_uintptr_t sp) { diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c index 0b425ca4975b..44022234b49b 100644 --- a/arch/arm64/kernel/process.c +++ b/arch/arm64/kernel/process.c @@ -674,7 +674,7 @@ void arch_setup_new_exec(void) }
if (system_supports_morello()) - morello_thread_init_user(current); + morello_thread_init_user(); }
#ifdef CONFIG_ARM64_TAGGED_ADDR_ABI diff --git a/arch/arm64/lib/morello.S b/arch/arm64/lib/morello.S index e250ff3dfc49..e1cd14355721 100644 --- a/arch/arm64/lib/morello.S +++ b/arch/arm64/lib/morello.S @@ -67,11 +67,9 @@ SYM_FUNC_START(morello_capcpy) ret SYM_FUNC_END(morello_capcpy)
-SYM_FUNC_START(morello_thread_init_user) +SYM_FUNC_START(__morello_thread_init_user) mov x9, #THREAD_MORELLO_USER_STATE add x0, x0, x9 // x0 = tsk->thread.morello_user_state - adr_l x1, morello_root_cap - ldr c1, [x1]
/* * CTPIDR doesn't need to be initialised explicitly: @@ -86,7 +84,7 @@ SYM_FUNC_START(morello_thread_init_user) */ msr rctpidr_el0, czr
- /* DDC: initialised to the root capability (like PCC) */ + /* DDC: initialised to the specified value */ msr ddc_el0, c1 /* RDDC: null capability (processes are always started in Executive) */ msr rddc_el0, czr @@ -99,7 +97,7 @@ SYM_FUNC_START(morello_thread_init_user) str xzr, [x0, #MORELLO_STATE_CCTLR]
ret -SYM_FUNC_END(morello_thread_init_user) +SYM_FUNC_END(__morello_thread_init_user)
SYM_FUNC_START(morello_thread_save_user_state) mov x9, #THREAD_MORELLO_USER_STATE
As a privileged operation (disabled by default), arbitrary capabilities may be created in a process via ptrace. Now that we have defined generic user root capabilities, make use of cheri_user_root_all_cap (the "root of roots") to build these capabilities from, instead of the kernel root capability. Change the name of the helper accordingly and clarify that it should not be used for unprivileged operations.
Using cheri_user_root_all_cap is a clear improvement, however this still allows creating capabilities that could not otherwise be obtained in PCuABI (e.g. with Seal combined with other permissions). Considering the additional complexity that restricting this further would entail, and the operation being only involved in privileged debugging, this situation is considered acceptable.
Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com --- arch/arm64/include/asm/morello.h | 10 +++++++++- arch/arm64/kernel/morello.c | 4 ++-- arch/arm64/kernel/ptrace.c | 2 +- arch/arm64/lib/morello.S | 9 +++------ 4 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/arch/arm64/include/asm/morello.h b/arch/arm64/include/asm/morello.h index e60ad5727fca..40a51fbe06d6 100644 --- a/arch/arm64/include/asm/morello.h +++ b/arch/arm64/include/asm/morello.h @@ -28,7 +28,15 @@ struct morello_state { };
void morello_cap_get_val_tag(uintcap_t cap, __uint128_t *val, u8 *tag); -uintcap_t morello_build_cap_from_root_cap(const __uint128_t *val, u8 tag); + +/* + * Builds a user capability from a 128-bit pattern + tag. The capability will + * be derived from cheri_user_root_all_cap and the object type will be + * preserved. + * + * This function should only be used for privileged operations (e.g. debug). + */ +uintcap_t morello_build_any_user_cap(const __uint128_t *val, u8 tag);
uintcap_t morello_get_root_cap(void);
diff --git a/arch/arm64/kernel/morello.c b/arch/arm64/kernel/morello.c index 9dc42231df20..ab83eb407eb9 100644 --- a/arch/arm64/kernel/morello.c +++ b/arch/arm64/kernel/morello.c @@ -203,8 +203,8 @@ static int access_remote_cap(struct task_struct *tsk, struct mm_struct *mm, goto out_put; }
- *kaddr = morello_build_cap_from_root_cap(&user_cap->val, - user_cap->tag); + *kaddr = morello_build_any_user_cap(&user_cap->val, + user_cap->tag); flush_ptrace_access(vma, (unsigned long)kaddr, (unsigned long)kaddr + sizeof(uintcap_t)); set_page_dirty_lock(page); diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c index db29bbe5ceae..6e3f6dc9c6f5 100644 --- a/arch/arm64/kernel/ptrace.c +++ b/arch/arm64/kernel/ptrace.c @@ -1167,7 +1167,7 @@ static int morello_get(struct task_struct *target,
#define MORELLO_STATE_BUILD_CAP(state, reg, cap) do { \ u8 tag = (state.tag_map >> MORELLO_PT_TAG_MAP_REG_BIT(reg)) & 0x1; \ - cap = morello_build_cap_from_root_cap(&state.reg, tag); \ + cap = morello_build_any_user_cap(&state.reg, tag); \ } while (0)
static int morello_set(struct task_struct *target, diff --git a/arch/arm64/lib/morello.S b/arch/arm64/lib/morello.S index e1cd14355721..62e8a6235134 100644 --- a/arch/arm64/lib/morello.S +++ b/arch/arm64/lib/morello.S @@ -22,7 +22,7 @@ SYM_FUNC_START(morello_cap_get_val_tag) ret SYM_FUNC_END(morello_cap_get_val_tag)
-SYM_FUNC_START(morello_build_cap_from_root_cap) +SYM_FUNC_START(morello_build_any_user_cap) ldr c0, [x0] cbz w1, 1f /* @@ -31,11 +31,8 @@ SYM_FUNC_START(morello_build_cap_from_root_cap) * In case c0 is sealed (non-zero object type), we need to extract the * object type first to be able to reseal it after BUILD. The CSEAL * instruction is used to cover the case where c0 was not sealed. - * - * TODO [PCuABI] - the user root capability should be used, not the - * kernel one. */ - adr_l x3, morello_root_cap + adr_l x3, cheri_user_root_all_cap ldr c3, [x3]
cpytype c4, c3, c0 @@ -47,7 +44,7 @@ SYM_FUNC_START(morello_build_cap_from_root_cap) clrtag c0, c0 2: ret -SYM_FUNC_END(morello_build_cap_from_root_cap) +SYM_FUNC_END(morello_build_any_user_cap)
SYM_FUNC_START(morello_capcpy) mov x3, x0
All users of morello_root_cap have now been moved to the appropriate cheri_user_root*_cap, and it is time to retire it.
Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com --- arch/arm64/include/asm/morello.h | 2 -- arch/arm64/kernel/morello.c | 10 ---------- 2 files changed, 12 deletions(-)
diff --git a/arch/arm64/include/asm/morello.h b/arch/arm64/include/asm/morello.h index 40a51fbe06d6..9b1117b9d227 100644 --- a/arch/arm64/include/asm/morello.h +++ b/arch/arm64/include/asm/morello.h @@ -38,8 +38,6 @@ void morello_cap_get_val_tag(uintcap_t cap, __uint128_t *val, u8 *tag); */ uintcap_t morello_build_any_user_cap(const __uint128_t *val, u8 tag);
-uintcap_t morello_get_root_cap(void); - /* * Reads or writes a capability from/to tsk's address space (depending on * gup_flags & FOLL_WRITE). diff --git a/arch/arm64/kernel/morello.c b/arch/arm64/kernel/morello.c index ab83eb407eb9..513b37865e61 100644 --- a/arch/arm64/kernel/morello.c +++ b/arch/arm64/kernel/morello.c @@ -27,19 +27,12 @@ void __morello_merge_c_x(uintcap_t *creg, u64 xreg); bool __morello_cap_has_executive(uintcap_t cap); void __morello_thread_init_user(struct task_struct *tsk, uintcap_t ddc);
-/* Not defined as static because morello.S refers to it */ -uintcap_t morello_root_cap __ro_after_init; static uintcap_t morello_sentry_unsealcap __ro_after_init;
/* DDC_ELx reset value (low/high 64 bits), as defined in the Morello spec */ #define DDC_RESET_VAL_LOW_64 0x0 #define DDC_RESET_VAL_HIGH_64 0xffffc00000010005ULL
-uintcap_t morello_get_root_cap(void) -{ - return morello_root_cap; -} - static bool is_pure_task(void) { return IS_ENABLED(CONFIG_CHERI_PURECAP_UABI) && !is_compat_task(); @@ -351,9 +344,6 @@ static int __init morello_cap_init(void) ARM_CAP_PERMISSION_COMPARTMENT_ID); cheri_user_root_cid_cap = ctemp;
- /* Initialise Morello-specific root capabilities. */ - morello_root_cap = root_cap; - /* Initialize a capability able to unseal sentry capabilities. */ ctemp = cheri_address_set(root_cap, CHERI_OTYPE_SENTRY); ctemp = cheri_bounds_set(ctemp, 1);
Update the documentation in line with the move to cheri_user_root_all_cap to derive all user capabilities from in hybrid, instead of the reset DDC.
Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com --- Documentation/arm64/morello.rst | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/Documentation/arm64/morello.rst b/Documentation/arm64/morello.rst index bc0d98596762..0686245c8381 100644 --- a/Documentation/arm64/morello.rst +++ b/Documentation/arm64/morello.rst @@ -5,7 +5,7 @@ Morello in AArch64 Linux Author: Kevin Brodsky kevin.brodsky@arm.com
| Original date: 2020-09-07 -| Last updated: 2022-04-22 +| Last updated: 2022-12-07 |
This document describes the provision of Morello functionalities to @@ -293,8 +293,7 @@ are initialized as follows:
* For capability registers, the upper 64 bits and tag are set to:
- - CMAX for PCC and DDC_EL0, as defined in the architecture (tag set, - maximum bounds, maximum permissions, object type set to 0). + - CROOT for PCC and DDC_EL0, as defined below. - All zeroes for all other registers.
* For capability registers, the lower 64 bits are set to: @@ -307,10 +306,18 @@ are initialized as follows:
* CCTLR_EL0 is set to 0.
+CROOT corresponds to the following capability attributes: + +* Tag set. +* Object type set to 0. +* Bounds including the entire user address space (whose size depends on + ``CONFIG_ARM64_VA_BITS``). +* All hardware-defined permissions and the User[0] permission. + Note - PCC has all permissions set after ``execve()``, which means that a - process is always started in Executive. All Restricted registers are - zeroed. + This means in particular that PCC is initialized with the Executive + permission set; as a result a process is always started in Executive. All + Restricted registers are zeroed.
Register merging principle ^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -484,14 +491,14 @@ Signal handling
When a signal handler is invoked:
-* PCC is reset to CMAX (see Generalities_ in the Register handling +* PCC is reset to CROOT (see Generalities_ in the Register handling section), and its address is set as usual to the signal handler's. This means in particular that **signal handlers are always run in Executive**. Accordingly, the signal frame is stored on the Executive stack (i.e. through CSP_EL0), if the alternate signal stack is not used.
-* CLR (C30) is also reset to CMAX, and its address set as usual (to the +* CLR (C30) is also reset to CROOT, and its address set as usual (to the signal trampoline). This allows a signal handler to return to the trampoline using the ``ret clr`` instruction, in addition to the usual ``ret [lr]`` instruction.
On 12/12/22 16:05, Kevin Brodsky wrote:
Hi,
This series is a follow-up to the RFC "New CHERI API and rehauled user_ptr.h", with a slightly different scope to make it more self-contained.
There are two main focuses for this series:
- Introducing linux/cheri.h. There is no fundamental change compared to v1 here.
- Deriving all capabilities from an appropriate userspace root capability (cheri_user_root_*) instead of morello_root_cap. v1 started this by reimplementing uaddr_to_user_ptr*, this series finishes up the work.
The focus of v1, adding generic functions to linux/user_ptr.h, has been dropped and will reappear in a separate series.
Some more details on the choice of root capabilities (see the comment in patch 5 regarding cheri_user_root_*):
In purecap, the PCuABI spec gives us good guidance on which root capability we should use where. Namely:
cheri_user_root_cap for almost all capabilities. The permissions correspond to the maximum permissions obtainable via mmap(). As we progress through the second phase, the bounds/permissions of capabilities derived from this root will be restricted as specified, and DDC will be set to null.
cheri_user_root_{seal,cid}_cap for the AT_CHERI_{SEAL,CID}_CAP. These capabilities exist precisely because their permissions (Seal/Unseal/CompartmentID) are not provided in regular capabilities (derived from cheri_user_root_cap).
cheri_user_root_all_cap for capabilities created via (privileged) ptrace. See patch 13 for some details on this.
In hybrid, the de facto ABI is what Documentation/arm64/morello.rst says. As there is no mechanism to obtain special permissions, all capabilities are derived from cheri_user_root_all_cap. The documentation is updated accordingly.
This series introduces functional changes by restricting the bounds/permissions of all userspace capabilities, but these restrictions should not affect any valid use-case. More specifically:
In purecap, the bounds of all capabilities are restricted to the user address space. See above for details on permissions.
In hybrid, the bounds of capabilities are also restricted to the user address space. All relevant permissions remain available. CSP is no longer initialised to a valid capability, as this is neither required nor documented.
I had a look at the v2 series. The series looks good to me and there are just few nits from my side.
Thanks, Amit Daniel
More detailed changelog below.
v1..v2:
- Addressing review comments:
- Reformatted the function documentation to make kernel-doc -v (mostly) happy.
- Added some comment clarifying what CHERI_PERM_SW_VMEM is about.
- Renamed ARCH_HAS_CHERI_H to HAVE_ARCH_CHERI_H.
- Renamed cheri_root*_cap_userspace to cheri_user_root_*cap and added some description of each.
- Renamed cheri_check_cap_data_access() to cheri_check_cap().
- New patches:
- Derive compat_ptr() from cheri_user_root_all_cap (deriving from DDC proved more complicated than expected, created a ticket for that [1])
- Derive AT_CHERI_{SEAL,CID}_CAP from cheri_user_root_{seal,cid}_cap
- Initialisation of capability registers from cheri_user_root_* (with a clear separation between purecap and hybrid)
- Capabilities created via (privileged) ptrace now derived from cheri_user_root_all_cap
- Remove morello_root_cap (no longer used)
- Update documentation to reflect cheri_user_root_all_cap being the new root capability in hybrid
- Other changes:
- As per a recent update to the PCuABI spec, the BranchSealedPair is no longer part of the rootcap permission set. It is still needed in certain user capabilities, so moved it from CHERI_PERMS_ROOTCAP to explicit addition to cheri_user_root_cap in morello.c.
- Added cheri_user_root_all_cap, the "root of roots" with all permissions. cheri_user_root_cid_cap is now derived from it too, so its bounds are not the whole address space any more.
- Patch 8/9 (new functions in user_ptr.h) dropped.
- Rebased on next.
Review branch:
https://git.morello-project.org/kbrodsky-arm/linux/-/commits/cheri_ptr_api_v...
Thanks, Kevin
[1] https://git.morello-project.org/morello/kernel/linux/-/issues/40
Kevin Brodsky (15): pps: Add missing #include linux/user_ptr.h: Remove kaddr_to_user_ptr() linux/user_ptr.h: Improve comment formatting arm64: uapi: Add asm/cheri.h linux/cheri.h: Introduce CHERI helpers arm64: morello: Implement cheri.h fs/binfmt_elf: Use appropriate caps for AT_CHERI_{SEAL,CID}_CAP arm64: compat: Use appropriate root cap in compat_ptr() in PCuABI linux/user_ptr.h: Generic PCuABI impl for uaddr_to_user_ptr* arm64: Remove asm/user_ptr.h arm64: morello: Initialise user capabilities from cheri_user_root_* arm64: morello: Initialise user DDC from cheri_user_root_* arm64: morello: Build arbitrary user caps using appropriate root arm64: morello: Remove morello_root_cap arm64: morello: Update root capability in documentation
Documentation/arm64/morello.rst | 23 +++-- Documentation/core-api/user_ptr.rst | 8 -- arch/Kconfig | 2 +- arch/arm64/Kconfig | 2 +- arch/arm64/include/asm/cheri.h | 11 +++ arch/arm64/include/asm/compat.h | 9 +- arch/arm64/include/asm/morello.h | 12 ++- arch/arm64/include/asm/user_ptr.h | 43 --------- arch/arm64/include/uapi/asm/cheri.h | 11 +++ arch/arm64/kernel/morello.c | 143 +++++++++++++++++----------- arch/arm64/kernel/process.c | 2 +- arch/arm64/kernel/ptrace.c | 2 +- arch/arm64/lib/morello.S | 17 ++-- drivers/pps/pps.c | 1 + fs/binfmt_elf.c | 10 +- include/linux/cheri.h | 132 +++++++++++++++++++++++++ include/linux/user_ptr.h | 69 ++++++-------- lib/Makefile | 3 + lib/cheri.c | 72 ++++++++++++++ lib/user_ptr.c | 26 +++++ 20 files changed, 413 insertions(+), 185 deletions(-) create mode 100644 arch/arm64/include/asm/cheri.h delete mode 100644 arch/arm64/include/asm/user_ptr.h create mode 100644 arch/arm64/include/uapi/asm/cheri.h create mode 100644 include/linux/cheri.h create mode 100644 lib/cheri.c create mode 100644 lib/user_ptr.c
On Mon, Dec 12, 2022 at 10:35:35AM +0000, Kevin Brodsky wrote:
Hi,
This series is a follow-up to the RFC "New CHERI API and rehauled user_ptr.h", with a slightly different scope to make it more self-contained.
There are two main focuses for this series:
- Introducing linux/cheri.h. There is no fundamental change compared to v1 here.
- Deriving all capabilities from an appropriate userspace root capability (cheri_user_root_*) instead of morello_root_cap. v1 started this by reimplementing uaddr_to_user_ptr*, this series finishes up the work.
The focus of v1, adding generic functions to linux/user_ptr.h, has been dropped and will reappear in a separate series.
Some more details on the choice of root capabilities (see the comment in patch 5 regarding cheri_user_root_*):
In purecap, the PCuABI spec gives us good guidance on which root capability we should use where. Namely:
cheri_user_root_cap for almost all capabilities. The permissions correspond to the maximum permissions obtainable via mmap(). As we progress through the second phase, the bounds/permissions of capabilities derived from this root will be restricted as specified, and DDC will be set to null.
cheri_user_root_{seal,cid}_cap for the AT_CHERI_{SEAL,CID}_CAP. These capabilities exist precisely because their permissions (Seal/Unseal/CompartmentID) are not provided in regular capabilities (derived from cheri_user_root_cap).
cheri_user_root_all_cap for capabilities created via (privileged) ptrace. See patch 13 for some details on this.
In hybrid, the de facto ABI is what Documentation/arm64/morello.rst says. As there is no mechanism to obtain special permissions, all capabilities are derived from cheri_user_root_all_cap. The documentation is updated accordingly.
This series introduces functional changes by restricting the bounds/permissions of all userspace capabilities, but these restrictions should not affect any valid use-case. More specifically:
In purecap, the bounds of all capabilities are restricted to the user address space. See above for details on permissions.
In hybrid, the bounds of capabilities are also restricted to the user address space. All relevant permissions remain available. CSP is no longer initialised to a valid capability, as this is neither required nor documented.
More detailed changelog below.
v1..v2:
- Addressing review comments:
- Reformatted the function documentation to make kernel-doc -v (mostly) happy.
- Added some comment clarifying what CHERI_PERM_SW_VMEM is about.
- Renamed ARCH_HAS_CHERI_H to HAVE_ARCH_CHERI_H.
- Renamed cheri_root*_cap_userspace to cheri_user_root_*cap and added some description of each.
- Renamed cheri_check_cap_data_access() to cheri_check_cap().
- New patches:
- Derive compat_ptr() from cheri_user_root_all_cap (deriving from DDC proved more complicated than expected, created a ticket for that [1])
- Derive AT_CHERI_{SEAL,CID}_CAP from cheri_user_root_{seal,cid}_cap
- Initialisation of capability registers from cheri_user_root_* (with a clear separation between purecap and hybrid)
- Capabilities created via (privileged) ptrace now derived from cheri_user_root_all_cap
- Remove morello_root_cap (no longer used)
- Update documentation to reflect cheri_user_root_all_cap being the new root capability in hybrid
- Other changes:
- As per a recent update to the PCuABI spec, the BranchSealedPair is no longer part of the rootcap permission set. It is still needed in certain user capabilities, so moved it from CHERI_PERMS_ROOTCAP to explicit addition to cheri_user_root_cap in morello.c.
- Added cheri_user_root_all_cap, the "root of roots" with all permissions. cheri_user_root_cid_cap is now derived from it too, so its bounds are not the whole address space any more.
- Patch 8/9 (new functions in user_ptr.h) dropped.
- Rebased on next.
Review branch:
https://git.morello-project.org/kbrodsky-arm/linux/-/commits/cheri_ptr_api_v...
Overall the series looks really good, just had few very minor comments on the way.
--- BR B.
Thanks, Kevin
[1] https://git.morello-project.org/morello/kernel/linux/-/issues/40
Kevin Brodsky (15): pps: Add missing #include linux/user_ptr.h: Remove kaddr_to_user_ptr() linux/user_ptr.h: Improve comment formatting arm64: uapi: Add asm/cheri.h linux/cheri.h: Introduce CHERI helpers arm64: morello: Implement cheri.h fs/binfmt_elf: Use appropriate caps for AT_CHERI_{SEAL,CID}_CAP arm64: compat: Use appropriate root cap in compat_ptr() in PCuABI linux/user_ptr.h: Generic PCuABI impl for uaddr_to_user_ptr* arm64: Remove asm/user_ptr.h arm64: morello: Initialise user capabilities from cheri_user_root_* arm64: morello: Initialise user DDC from cheri_user_root_* arm64: morello: Build arbitrary user caps using appropriate root arm64: morello: Remove morello_root_cap arm64: morello: Update root capability in documentation
Documentation/arm64/morello.rst | 23 +++-- Documentation/core-api/user_ptr.rst | 8 -- arch/Kconfig | 2 +- arch/arm64/Kconfig | 2 +- arch/arm64/include/asm/cheri.h | 11 +++ arch/arm64/include/asm/compat.h | 9 +- arch/arm64/include/asm/morello.h | 12 ++- arch/arm64/include/asm/user_ptr.h | 43 --------- arch/arm64/include/uapi/asm/cheri.h | 11 +++ arch/arm64/kernel/morello.c | 143 +++++++++++++++++----------- arch/arm64/kernel/process.c | 2 +- arch/arm64/kernel/ptrace.c | 2 +- arch/arm64/lib/morello.S | 17 ++-- drivers/pps/pps.c | 1 + fs/binfmt_elf.c | 10 +- include/linux/cheri.h | 132 +++++++++++++++++++++++++ include/linux/user_ptr.h | 69 ++++++-------- lib/Makefile | 3 + lib/cheri.c | 72 ++++++++++++++ lib/user_ptr.c | 26 +++++ 20 files changed, 413 insertions(+), 185 deletions(-) create mode 100644 arch/arm64/include/asm/cheri.h delete mode 100644 arch/arm64/include/asm/user_ptr.h create mode 100644 arch/arm64/include/uapi/asm/cheri.h create mode 100644 include/linux/cheri.h create mode 100644 lib/cheri.c create mode 100644 lib/user_ptr.c
-- 2.38.1
linux-morello mailing list -- linux-morello@op-lists.linaro.org To unsubscribe send an email to linux-morello-leave@op-lists.linaro.org
On 31/01/2023 12:34, Beata Michalska wrote:
Review branch:
https://git.morello-project.org/kbrodsky-arm/linux/-/commits/cheri_ptr_api_v...
Overall the series looks really good, just had few very minor comments on the way.
Thanks for the review, very much appreciated!
Kevin
linux-morello@op-lists.linaro.org
-
Amit Daniel Kachhap -
Beata Michalska -
Kevin Brodsky