Skip to content

Commit d60624f

Browse files
mrutland-armctmarinas
authored andcommitted
arm64: ptrace: fix partial SETREGSET for NT_ARM_GCS
Currently gcs_set() doesn't initialize the temporary 'user_gcs' variable, and a SETREGSET call with a length of 0, 8, or 16 will leave some portion of this uninitialized. Consequently some arbitrary uninitialized values may be written back to the relevant fields in task struct, potentially leaking up to 192 bits of memory from the kernel stack. The read is limited to a specific slot on the stack, and the issue does not provide a write mechanism. As gcs_set() rejects cases where user_gcs::features_enabled has bits set other than PR_SHADOW_STACK_SUPPORTED_STATUS_MASK, a SETREGSET call with a length of zero will randomly succeed or fail depending on the value of the uninitialized value, it isn't possible to leak the full 192 bits. With a length of 8 or 16, user_gcs::features_enabled can be initialized to an accepted value, making it practical to leak 128 or 64 bits. Fix this by initializing the temporary value before copying the regset from userspace, as for other regsets (e.g. NT_PRSTATUS, NT_PRFPREG, NT_ARM_SYSTEM_CALL). In the case of a zero-length or partial write, the existing contents of the fields which are not written to will be retained. To ensure that the extraction and insertion of fields is consistent across the GETREGSET and SETREGSET calls, new task_gcs_to_user() and task_gcs_from_user() helpers are added, matching the style of pac_address_keys_to_user() and pac_address_keys_from_user(). Before this patch: | # ./gcs-test | Attempting to write NT_ARM_GCS::user_gcs = { | .features_enabled = 0x0000000000000000, | .features_locked = 0x0000000000000000, | .gcspr_el0 = 0x900d900d900d900d, | } | SETREGSET(nt=0x410, len=24) wrote 24 bytes | | Attempting to read NT_ARM_GCS::user_gcs | GETREGSET(nt=0x410, len=24) read 24 bytes | Read NT_ARM_GCS::user_gcs = { | .features_enabled = 0x0000000000000000, | .features_locked = 0x0000000000000000, | .gcspr_el0 = 0x900d900d900d900d, | } | | Attempting partial write NT_ARM_GCS::user_gcs = { | .features_enabled = 0x0000000000000000, | .features_locked = 0x1de7ec7edbadc0de, | .gcspr_el0 = 0x1de7ec7edbadc0de, | } | SETREGSET(nt=0x410, len=8) wrote 8 bytes | | Attempting to read NT_ARM_GCS::user_gcs | GETREGSET(nt=0x410, len=24) read 24 bytes | Read NT_ARM_GCS::user_gcs = { | .features_enabled = 0x0000000000000000, | .features_locked = 0x000000000093e780, | .gcspr_el0 = 0xffff800083a63d50, | } After this patch: | # ./gcs-test | Attempting to write NT_ARM_GCS::user_gcs = { | .features_enabled = 0x0000000000000000, | .features_locked = 0x0000000000000000, | .gcspr_el0 = 0x900d900d900d900d, | } | SETREGSET(nt=0x410, len=24) wrote 24 bytes | | Attempting to read NT_ARM_GCS::user_gcs | GETREGSET(nt=0x410, len=24) read 24 bytes | Read NT_ARM_GCS::user_gcs = { | .features_enabled = 0x0000000000000000, | .features_locked = 0x0000000000000000, | .gcspr_el0 = 0x900d900d900d900d, | } | | Attempting partial write NT_ARM_GCS::user_gcs = { | .features_enabled = 0x0000000000000000, | .features_locked = 0x1de7ec7edbadc0de, | .gcspr_el0 = 0x1de7ec7edbadc0de, | } | SETREGSET(nt=0x410, len=8) wrote 8 bytes | | Attempting to read NT_ARM_GCS::user_gcs | GETREGSET(nt=0x410, len=24) read 24 bytes | Read NT_ARM_GCS::user_gcs = { | .features_enabled = 0x0000000000000000, | .features_locked = 0x0000000000000000, | .gcspr_el0 = 0x900d900d900d900d, | } Fixes: 7ec3b57 ("arm64/ptrace: Expose GCS via ptrace and core files") Signed-off-by: Mark Rutland <[email protected]> Cc: Mark Brown <[email protected]> Cc: Will Deacon <[email protected]> Reviewed-by: Mark Brown <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Catalin Marinas <[email protected]>
1 parent 594bfc4 commit d60624f

File tree

1 file changed

+20
-6
lines changed

1 file changed

+20
-6
lines changed

arch/arm64/kernel/ptrace.c

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1491,6 +1491,22 @@ static int poe_set(struct task_struct *target, const struct
14911491
#endif
14921492

14931493
#ifdef CONFIG_ARM64_GCS
1494+
static void task_gcs_to_user(struct user_gcs *user_gcs,
1495+
const struct task_struct *target)
1496+
{
1497+
user_gcs->features_enabled = target->thread.gcs_el0_mode;
1498+
user_gcs->features_locked = target->thread.gcs_el0_locked;
1499+
user_gcs->gcspr_el0 = target->thread.gcspr_el0;
1500+
}
1501+
1502+
static void task_gcs_from_user(struct task_struct *target,
1503+
const struct user_gcs *user_gcs)
1504+
{
1505+
target->thread.gcs_el0_mode = user_gcs->features_enabled;
1506+
target->thread.gcs_el0_locked = user_gcs->features_locked;
1507+
target->thread.gcspr_el0 = user_gcs->gcspr_el0;
1508+
}
1509+
14941510
static int gcs_get(struct task_struct *target,
14951511
const struct user_regset *regset,
14961512
struct membuf to)
@@ -1503,9 +1519,7 @@ static int gcs_get(struct task_struct *target,
15031519
if (target == current)
15041520
gcs_preserve_current_state();
15051521

1506-
user_gcs.features_enabled = target->thread.gcs_el0_mode;
1507-
user_gcs.features_locked = target->thread.gcs_el0_locked;
1508-
user_gcs.gcspr_el0 = target->thread.gcspr_el0;
1522+
task_gcs_to_user(&user_gcs, target);
15091523

15101524
return membuf_write(&to, &user_gcs, sizeof(user_gcs));
15111525
}
@@ -1521,16 +1535,16 @@ static int gcs_set(struct task_struct *target, const struct
15211535
if (!system_supports_gcs())
15221536
return -EINVAL;
15231537

1538+
task_gcs_to_user(&user_gcs, target);
1539+
15241540
ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, &user_gcs, 0, -1);
15251541
if (ret)
15261542
return ret;
15271543

15281544
if (user_gcs.features_enabled & ~PR_SHADOW_STACK_SUPPORTED_STATUS_MASK)
15291545
return -EINVAL;
15301546

1531-
target->thread.gcs_el0_mode = user_gcs.features_enabled;
1532-
target->thread.gcs_el0_locked = user_gcs.features_locked;
1533-
target->thread.gcspr_el0 = user_gcs.gcspr_el0;
1547+
task_gcs_from_user(target, &user_gcs);
15341548

15351549
return 0;
15361550
}

0 commit comments

Comments
 (0)