| Message ID | 20231122-arm64-gcs-v7-24-201c483bd775@kernel.org (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | arm64/gcs: Provide support for GCS in userspace | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/vmtest-fixes-PR | fail | merge-conflict |
Mark Brown <broonie@kernel.org> writes: > +static bool gcs_signal_cap_valid(u64 addr, u64 val) > +{ > + /* > + * The top bit should be set, this is an invalid address for > + * EL0 and will only be set for caps created by signals. > + */ > + if (!(val & GCS_SIGNAL_CAP_FLAG)) > + return false; > + > + /* The rest should be a standard architectural cap token. */ > + val &= ~GCS_SIGNAL_CAP_FLAG; > + > + /* The cap must have the low bits set to a token value */ > + if (GCS_CAP_TOKEN(val) != 0) > + return false; I found the comment above a little confusing, since the if condition actually checks that low bits aren't set at all. Perhaps reword to something like "The token value of a signal cap must be 0"? > + > + /* The cap must store the VA the cap was stored at */ > + if (GCS_CAP_ADDR(addr) != GCS_CAP_ADDR(val)) > + return false; > + > + return true; > +} > +#endif > + > /* > * Do a signal return; undo the signal stack. These are aligned to 128-bit. > */ > @@ -815,6 +847,45 @@ static int restore_sigframe(struct pt_regs *regs, > return err; > } > > +#ifdef CONFIG_ARM64_GCS > +static int gcs_restore_signal(void) > +{ > + u64 gcspr_el0, cap; > + int ret; > + > + if (!system_supports_gcs()) > + return 0; > + > + if (!(current->thread.gcs_el0_mode & PR_SHADOW_STACK_ENABLE)) > + return 0; > + > + gcspr_el0 = read_sysreg_s(SYS_GCSPR_EL0); > + > + /* > + * GCSPR_EL0 should be pointing at a capped GCS, read the cap... > + */ > + gcsb_dsync(); > + ret = copy_from_user(&cap, (__user void*)gcspr_el0, sizeof(cap)); > + if (ret) > + return -EFAULT; > + > + /* > + * ...then check that the cap is the actual GCS before > + * restoring it. > + */ > + if (!gcs_signal_cap_valid(gcspr_el0, cap)) > + return -EINVAL; > + > + current->thread.gcspr_el0 = gcspr_el0 + sizeof(cap); > + write_sysreg_s(current->thread.gcspr_el0, SYS_GCSPR_EL0); At this point, there's an inactive but valid cap just below the GCS. Over time, as different signals are received when the GCSPR is pointing at different locations of the stack, there could be a number of valid inactive caps available for misuse. I'm still not proficient enough in GCS to know how exactly this could be abused (e.g., somehow writing the desired return location right above one of these inactive caps and arranging for GCSPR to point to the cap before returning from a signal) but to be safe or paranoid, perhaps zero the location of the cap before returning? > + > + return 0; > +} > + > +#else > +static int gcs_restore_signal(void) { return 0; } > +#endif > + > SYSCALL_DEFINE0(rt_sigreturn) > { > struct pt_regs *regs = current_pt_regs(); > @@ -841,6 +912,9 @@ SYSCALL_DEFINE0(rt_sigreturn) > if (restore_altstack(&frame->uc.uc_stack)) > goto badframe; > > + if (gcs_restore_signal()) > + goto badframe; > + > return regs->regs[0]; > > badframe: > @@ -1071,7 +1145,50 @@ static int get_sigframe(struct rt_sigframe_user_layout *user, > return 0; > } > > -static void setup_return(struct pt_regs *regs, struct k_sigaction *ka, > +#ifdef CONFIG_ARM64_GCS > + > +static int gcs_signal_entry(__sigrestore_t sigtramp, struct ksignal *ksig) The ksig argument is unused, so it can be removed. > +{ > + unsigned long __user *gcspr_el0; > + int ret = 0; > + > + if (!system_supports_gcs()) > + return 0; > + > + if (!task_gcs_el0_enabled(current)) > + return 0; > + > + /* > + * We are entering a signal handler, current register state is > + * active. > + */ > + gcspr_el0 = (unsigned long __user *)read_sysreg_s(SYS_GCSPR_EL0); > + > + /* > + * Push a cap and the GCS entry for the trampoline onto the GCS. > + */ > + put_user_gcs((unsigned long)sigtramp, gcspr_el0 - 2, &ret); > + put_user_gcs(GCS_SIGNAL_CAP(gcspr_el0 - 1), gcspr_el0 - 1, &ret); > + if (ret != 0) > + return ret; > + > + gcsb_dsync(); > + > + gcspr_el0 -= 2; > + write_sysreg_s((unsigned long)gcspr_el0, SYS_GCSPR_EL0); > + > + return 0; > +} > +#else > + > +static int gcs_signal_entry(__sigrestore_t sigtramp, struct ksignal *ksig) > +{ > + return 0; > +} > + > +#endif > + > +static int setup_return(struct pt_regs *regs, struct ksignal *ksig, > struct rt_sigframe_user_layout *user, int usig) Since the ksig argument isn't used by gcs_signal_entry(), setup_return() can keep the ka argument and the changes below from ka to ksic->ka are unnecessary. > { > __sigrestore_t sigtramp; > @@ -1079,7 +1196,7 @@ static void setup_return(struct pt_regs *regs, struct k_sigaction *ka, > regs->regs[0] = usig; > regs->sp = (unsigned long)user->sigframe; > regs->regs[29] = (unsigned long)&user->next_frame->fp; > - regs->pc = (unsigned long)ka->sa.sa_handler; > + regs->pc = (unsigned long)ksig->ka.sa.sa_handler; > > /* > * Signal delivery is a (wacky) indirect function call in > @@ -1119,12 +1236,14 @@ static void setup_return(struct pt_regs *regs, struct k_sigaction *ka, > sme_smstop(); > } > > - if (ka->sa.sa_flags & SA_RESTORER) > - sigtramp = ka->sa.sa_restorer; > + if (ksig->ka.sa.sa_flags & SA_RESTORER) > + sigtramp = ksig->ka.sa.sa_restorer; > else > sigtramp = VDSO_SYMBOL(current->mm->context.vdso, sigtramp); > > regs->regs[30] = (unsigned long)sigtramp; > + > + return gcs_signal_entry(sigtramp, ksig); > } > > static int setup_rt_frame(int usig, struct ksignal *ksig, sigset_t *set, > @@ -1147,7 +1266,7 @@ static int setup_rt_frame(int usig, struct ksignal *ksig, sigset_t *set, > err |= __save_altstack(&frame->uc.uc_stack, regs->sp); > err |= setup_sigframe(&user, regs, set); > if (err == 0) { > - setup_return(regs, &ksig->ka, &user, usig); > + err = setup_return(regs, ksig, &user, usig); > if (ksig->ka.sa.sa_flags & SA_SIGINFO) { > err |= copy_siginfo_to_user(&frame->info, &ksig->info); > regs->regs[1] = (unsigned long)&frame->info; > diff --git a/arch/arm64/mm/gcs.c b/arch/arm64/mm/gcs.c > index 02f8f6046c10..6f51429c5a46 100644 > --- a/arch/arm64/mm/gcs.c > +++ b/arch/arm64/mm/gcs.c > @@ -6,6 +6,7 @@ > #include <linux/types.h> > > #include <asm/cpufeature.h> > +#include <asm/gcs.h> > #include <asm/page.h> This is #include isn't needed by this patch. Probably better as part of another one.
On Sat, Dec 09, 2023 at 12:15:22AM -0300, Thiago Jung Bauermann wrote: > Mark Brown <broonie@kernel.org> writes: > > + /* The cap must have the low bits set to a token value */ > > + if (GCS_CAP_TOKEN(val) != 0) > > + return false; > I found the comment above a little confusing, since the if condition > actually checks that low bits aren't set at all. Perhaps reword to > something like "The token value of a signal cap must be 0"? Right, that's bitrot from the previous token format. > I'm still not proficient enough in GCS to know how exactly this could be > abused (e.g., somehow writing the desired return location right above > one of these inactive caps and arranging for GCSPR to point to the cap > before returning from a signal) but to be safe or paranoid, perhaps zero > the location of the cap before returning? Right, ideally we'd be doing a compare and exchange here to substitute in a zero.
diff --git a/arch/arm64/include/asm/gcs.h b/arch/arm64/include/asm/gcs.h index 48c97e63e56a..f50660603ecf 100644 --- a/arch/arm64/include/asm/gcs.h +++ b/arch/arm64/include/asm/gcs.h @@ -9,6 +9,7 @@ #include <asm/uaccess.h> struct kernel_clone_args; +struct ksignal; static inline void gcsb_dsync(void) { diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c index 0e8beb3349ea..b5dff91274fb 100644 --- a/arch/arm64/kernel/signal.c +++ b/arch/arm64/kernel/signal.c @@ -25,6 +25,7 @@ #include <asm/elf.h> #include <asm/exception.h> #include <asm/cacheflush.h> +#include <asm/gcs.h> #include <asm/ucontext.h> #include <asm/unistd.h> #include <asm/fpsimd.h> @@ -34,6 +35,37 @@ #include <asm/traps.h> #include <asm/vdso.h> +#ifdef CONFIG_ARM64_GCS +/* Extra bit set in the address distinguishing a signal cap token. */ +#define GCS_SIGNAL_CAP_FLAG BIT(63) + +#define GCS_SIGNAL_CAP(addr) ((((unsigned long)addr) & GCS_CAP_ADDR_MASK) | \ + GCS_SIGNAL_CAP_FLAG) + +static bool gcs_signal_cap_valid(u64 addr, u64 val) +{ + /* + * The top bit should be set, this is an invalid address for + * EL0 and will only be set for caps created by signals. + */ + if (!(val & GCS_SIGNAL_CAP_FLAG)) + return false; + + /* The rest should be a standard architectural cap token. */ + val &= ~GCS_SIGNAL_CAP_FLAG; + + /* The cap must have the low bits set to a token value */ + if (GCS_CAP_TOKEN(val) != 0) + return false; + + /* The cap must store the VA the cap was stored at */ + if (GCS_CAP_ADDR(addr) != GCS_CAP_ADDR(val)) + return false; + + return true; +} +#endif + /* * Do a signal return; undo the signal stack. These are aligned to 128-bit. */ @@ -815,6 +847,45 @@ static int restore_sigframe(struct pt_regs *regs, return err; } +#ifdef CONFIG_ARM64_GCS +static int gcs_restore_signal(void) +{ + u64 gcspr_el0, cap; + int ret; + + if (!system_supports_gcs()) + return 0; + + if (!(current->thread.gcs_el0_mode & PR_SHADOW_STACK_ENABLE)) + return 0; + + gcspr_el0 = read_sysreg_s(SYS_GCSPR_EL0); + + /* + * GCSPR_EL0 should be pointing at a capped GCS, read the cap... + */ + gcsb_dsync(); + ret = copy_from_user(&cap, (__user void*)gcspr_el0, sizeof(cap)); + if (ret) + return -EFAULT; + + /* + * ...then check that the cap is the actual GCS before + * restoring it. + */ + if (!gcs_signal_cap_valid(gcspr_el0, cap)) + return -EINVAL; + + current->thread.gcspr_el0 = gcspr_el0 + sizeof(cap); + write_sysreg_s(current->thread.gcspr_el0, SYS_GCSPR_EL0); + + return 0; +} + +#else +static int gcs_restore_signal(void) { return 0; } +#endif + SYSCALL_DEFINE0(rt_sigreturn) { struct pt_regs *regs = current_pt_regs(); @@ -841,6 +912,9 @@ SYSCALL_DEFINE0(rt_sigreturn) if (restore_altstack(&frame->uc.uc_stack)) goto badframe; + if (gcs_restore_signal()) + goto badframe; + return regs->regs[0]; badframe: @@ -1071,7 +1145,50 @@ static int get_sigframe(struct rt_sigframe_user_layout *user, return 0; } -static void setup_return(struct pt_regs *regs, struct k_sigaction *ka, +#ifdef CONFIG_ARM64_GCS + +static int gcs_signal_entry(__sigrestore_t sigtramp, struct ksignal *ksig) +{ + unsigned long __user *gcspr_el0; + int ret = 0; + + if (!system_supports_gcs()) + return 0; + + if (!task_gcs_el0_enabled(current)) + return 0; + + /* + * We are entering a signal handler, current register state is + * active. + */ + gcspr_el0 = (unsigned long __user *)read_sysreg_s(SYS_GCSPR_EL0); + + /* + * Push a cap and the GCS entry for the trampoline onto the GCS. + */ + put_user_gcs((unsigned long)sigtramp, gcspr_el0 - 2, &ret); + put_user_gcs(GCS_SIGNAL_CAP(gcspr_el0 - 1), gcspr_el0 - 1, &ret); + if (ret != 0) + return ret; + + gcsb_dsync(); + + gcspr_el0 -= 2; + write_sysreg_s((unsigned long)gcspr_el0, SYS_GCSPR_EL0); + + return 0; +} +#else + +static int gcs_signal_entry(__sigrestore_t sigtramp, struct ksignal *ksig) +{ + return 0; +} + +#endif + +static int setup_return(struct pt_regs *regs, struct ksignal *ksig, struct rt_sigframe_user_layout *user, int usig) { __sigrestore_t sigtramp; @@ -1079,7 +1196,7 @@ static void setup_return(struct pt_regs *regs, struct k_sigaction *ka, regs->regs[0] = usig; regs->sp = (unsigned long)user->sigframe; regs->regs[29] = (unsigned long)&user->next_frame->fp; - regs->pc = (unsigned long)ka->sa.sa_handler; + regs->pc = (unsigned long)ksig->ka.sa.sa_handler; /* * Signal delivery is a (wacky) indirect function call in @@ -1119,12 +1236,14 @@ static void setup_return(struct pt_regs *regs, struct k_sigaction *ka, sme_smstop(); } - if (ka->sa.sa_flags & SA_RESTORER) - sigtramp = ka->sa.sa_restorer; + if (ksig->ka.sa.sa_flags & SA_RESTORER) + sigtramp = ksig->ka.sa.sa_restorer; else sigtramp = VDSO_SYMBOL(current->mm->context.vdso, sigtramp); regs->regs[30] = (unsigned long)sigtramp; + + return gcs_signal_entry(sigtramp, ksig); } static int setup_rt_frame(int usig, struct ksignal *ksig, sigset_t *set, @@ -1147,7 +1266,7 @@ static int setup_rt_frame(int usig, struct ksignal *ksig, sigset_t *set, err |= __save_altstack(&frame->uc.uc_stack, regs->sp); err |= setup_sigframe(&user, regs, set); if (err == 0) { - setup_return(regs, &ksig->ka, &user, usig); + err = setup_return(regs, ksig, &user, usig); if (ksig->ka.sa.sa_flags & SA_SIGINFO) { err |= copy_siginfo_to_user(&frame->info, &ksig->info); regs->regs[1] = (unsigned long)&frame->info; diff --git a/arch/arm64/mm/gcs.c b/arch/arm64/mm/gcs.c index 02f8f6046c10..6f51429c5a46 100644 --- a/arch/arm64/mm/gcs.c +++ b/arch/arm64/mm/gcs.c @@ -6,6 +6,7 @@ #include <linux/types.h> #include <asm/cpufeature.h> +#include <asm/gcs.h> #include <asm/page.h> static unsigned long alloc_gcs(unsigned long addr, unsigned long size,
When invoking a signal handler we use the GCS configuration and stack for the current thread. Since we implement signal return by calling the signal handler with a return address set up pointing to a trampoline in the vDSO we need to also configure any active GCS for this by pushing a frame for the trampoline onto the GCS. If we do not do this then signal return will generate a GCS protection fault. In order to guard against attempts to bypass GCS protections via signal return we only allow returning with GCSPR_EL0 pointing to an address where it was previously preempted by a signal. We do this by pushing a cap onto the GCS, this takes the form of an architectural GCS cap token with the top bit set and token type of 0 which we add on signal entry and validate and pop off on signal return. The combination of the top bit being set and the token type mean that this can't be interpreted as a valid token or address. Signed-off-by: Mark Brown <broonie@kernel.org> --- arch/arm64/include/asm/gcs.h | 1 + arch/arm64/kernel/signal.c | 129 +++++++++++++++++++++++++++++++++++++++++-- arch/arm64/mm/gcs.c | 1 + 3 files changed, 126 insertions(+), 5 deletions(-)