On Wed, Mar 15, 2023 at 10:30:26AM +0100, Kevin Brodsky wrote:
On 08/03/2023 10:15, Beata Michalska wrote:
Hi Amit,
On Fri, Mar 03, 2023 at 06:34:41PM +0530, Amit Daniel Kachhap wrote:
Some of the Gcc compiled clone tests in restricted mode face stack corruption. This occurs as the dynamic relocations by Gcc do not get input from compiler about clearing the executive mode in function pointers. On the other hand, Clang provides input about this permission in the structure cap_reloc (may be with hint from instruction blrr) to the capability relocation code.
So I must admit I've been struggling a bit to connect the dots here. I'm not entirely sure how relocations would deal with executive vs restricted: two entries for the same symbol with different permissions ? Furthermore, the symbol at hand, in the case of clang is not actually being relocated. The branch capability is being build based on PCC, which for restricted renders proper capability permissions (as PCC will already have the executive bit cleared). Now with GCC, the symbol is relocated with executive permission which means the branch instruction will switch from restrictive back to executive which is were the problem comes from: that switch will also switch the SP (from RCSP to CSP) and as that one is not the one expected, it will result in number of issues. Note that the CSP will point to the stack from before switching to restricted. And that is the main problem here.
This is also my understanding. The commit message and comment should be updated accordingly.
Your change, even though it is actually fixing the mess we end up with GCC, it brings more generic question on how to actually handle restricted. Note that any branch to relocated symbol, without proper handling of related registers or permissions will blow up.
Agreed, this is pretty fragile and we just got lucky so far. That said I don't think we can make the test fundamentally more robust without changing the approach completely, so we'll have to live with workarounds.
I guess though, that's the best we can do at this point.
This approach is one option, though I think we can avoiding adding extra logic to __clone(), which IMHO should be as thin a wrapper as possible.
An alternative is essentially to materialise the function pointer ourselves, instead of relying on on Clang to do it for us, something like:
ptr = cheri_address_set(cheri_pcc_get(), (ptraddr_t)clone_base_fn); ptr = cheri_sentry_create(ptr);
This avoids adding logic in assembly, keeps __clone() untouched and doesn't rely on any particular behaviour from the compiler. It makes the assumption that the bounds and permissions of clone_base_fn are the same as PCC, which is not an issue in practice (being in the same translation unit).
+1 for this one.
--- BR B.
BR B.
As a workaround, clear this permission bit explicitly if required before branching to this function pointer. Also, keep any other function called from this function as inline.
I don't think that's necessary. Normal function calls are not capability branches, just a BL with an offset. We only have a problem when a full function pointer needs to be materialised.
Kevin
Signed-off-by: Amit Daniel Kachhap amit.kachhap@arm.com
tools/testing/selftests/arm64/morello/clone.c | 3 ++- .../arm64/morello/freestanding_start.S | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/arm64/morello/clone.c b/tools/testing/selftests/arm64/morello/clone.c index 69189287144a..67468cb524bc 100644 --- a/tools/testing/selftests/arm64/morello/clone.c +++ b/tools/testing/selftests/arm64/morello/clone.c @@ -44,7 +44,8 @@ struct test_fixture { #define PROBE_INTERVAL (1 << 12) -static void probe_addr_range(uintcap_t start_addr, size_t size, int interval) +static inline __attribute__((always_inline)) +void probe_addr_range(uintcap_t start_addr, size_t size, int interval) { size_t i; diff --git a/tools/testing/selftests/arm64/morello/freestanding_start.S b/tools/testing/selftests/arm64/morello/freestanding_start.S index 92260fdc384d..365a8d16606a 100644 --- a/tools/testing/selftests/arm64/morello/freestanding_start.S +++ b/tools/testing/selftests/arm64/morello/freestanding_start.S @@ -199,6 +199,24 @@ FUNCTION_START(__clone) 1: mov c0, c10 mov c1, c9
- /*
* Function pointers executed in restrictive mode have the executive* mode permission cleared off by the Clang capability relocation code.* However, dynamic relocations by GCC do not manage this, so clear this* permission bit explicitly if required.*/- adr c2, #0
- gcperm x3, c2
- tbnz x3, #__ARM_CAP_PERMISSION_EXECUTIVE__, 2f
- mov x4, #__ARM_CAP_PERMISSION_EXECUTIVE__
- clrperm c1, c1, x4
- /*
* Function pointers are sealed so clrperm untagged c1 - rebuild a valid* capability from PCC*/- build c1, c1, c2
+2: blr c1 mov x8, #__NR_exit svc #0 -- 2.25.1