lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sun, 19 Jun 2022 01:25:13 +0700 From: Bui Quang Minh <minhquangbui99@...il.com> To: linux-kernel@...r.kernel.org Cc: Bui Quang Minh <minhquangbui99@...il.com>, Alexander Mikhalitsyn <alexander@...alicyn.com>, Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org, "H. Peter Anvin" <hpa@...or.com>, Peter Zijlstra <peterz@...radead.org>, Juri Lelli <juri.lelli@...hat.com>, Vincent Guittot <vincent.guittot@...aro.org>, Dietmar Eggemann <dietmar.eggemann@....com>, Steven Rostedt <rostedt@...dmis.org>, Ben Segall <bsegall@...gle.com>, Mel Gorman <mgorman@...e.de>, Daniel Bristot de Oliveira <bristot@...hat.com>, Valentin Schneider <vschneid@...hat.com>, Mathieu Desnoyers <mathieu.desnoyers@...icios.com>, "Paul E. McKenney" <paulmck@...nel.org>, Boqun Feng <boqun.feng@...il.com>, Kees Cook <keescook@...omium.org>, "Chang S. Bae" <chang.seok.bae@...el.com>, "Eric W. Biederman" <ebiederm@...ssion.com>, Brian Gerst <brgerst@...il.com> Subject: [PATCH] rseq: x86: Fix rseq_cs get cleared when returning from signal handler rseq.rseq_cs is not expected to be cleared when we are still in critical section. When using rseq with RSEQ_CS_FLAG_NO_RESTART_ON_SIGNAL, if the signal is delivered, we don't restart to the abort ip but go to signal handler then return and continue the critical section execution. While in signal handler, if a preemption, migration or another signal is delivered, we will fall into rseq_ip_fixup again. At that moment, the IP is in signal handler not in critical section so the rseq.rseq_cs is cleared. Later, when we return back to critical section from signal handlers, we get the rseq.rseq_cs incorrectly cleared. Another scenario is when a preemption or migration happens, then a signal is delivered. When preparing to return to user mode, in exit_to_user_mode_loop, we first handle the signal and set return IP to signal handler. After that, rseq_handle_notify_resume is called to handle the preemption/migration, sees the IP is the address of signal handler not in critical section and clear rseq.rseq_cs. To handle this case, a RSEQ_CS_FLAG_IN_SIGNAL_HANDLER is added. This flag is set in rseq.flags (it is easier to read and update rseq.flags than rseq.rseq_cs.flags) by the kernel when rseq does not restart on signal. When this flag is set, we don't clear rseq.rseq_cs when the IP is outside of critical section. The flag is unset in sigreturn path when the last signal handler returns back to the critical section. Reviewed-by: Alexander Mikhalitsyn <alexander@...alicyn.com> Signed-off-by: Bui Quang Minh <minhquangbui99@...il.com> --- arch/x86/kernel/signal.c | 7 ++++ include/linux/sched.h | 5 +++ include/uapi/linux/rseq.h | 8 +++- kernel/rseq.c | 80 ++++++++++++++++++++++++++++++++++++--- 4 files changed, 94 insertions(+), 6 deletions(-) diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c index 9c7265b524c7..7aadf69d6849 100644 --- a/arch/x86/kernel/signal.c +++ b/arch/x86/kernel/signal.c @@ -646,6 +646,10 @@ SYSCALL_DEFINE0(sigreturn) */ if (!restore_sigcontext(regs, &frame->sc, 0)) goto badframe; + + if (rseq_sigreturn(regs)) + goto badframe; + return regs->ax; badframe: @@ -678,6 +682,9 @@ SYSCALL_DEFINE0(rt_sigreturn) if (restore_altstack(&frame->uc.uc_stack)) goto badframe; + if (rseq_sigreturn(regs)) + goto badframe; + return regs->ax; badframe: diff --git a/include/linux/sched.h b/include/linux/sched.h index c46f3a63b758..656b03232d0a 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -2340,6 +2340,8 @@ static inline void rseq_execve(struct task_struct *t) t->rseq_event_mask = 0; } +int rseq_sigreturn(struct pt_regs *regs); + #else static inline void rseq_set_notify_resume(struct task_struct *t) @@ -2365,6 +2367,9 @@ static inline void rseq_fork(struct task_struct *t, unsigned long clone_flags) static inline void rseq_execve(struct task_struct *t) { } +static inline int rseq_sigreturn(struct pt_regs *regs) +{ +} #endif diff --git a/include/uapi/linux/rseq.h b/include/uapi/linux/rseq.h index 77ee207623a9..f946af8e21e6 100644 --- a/include/uapi/linux/rseq.h +++ b/include/uapi/linux/rseq.h @@ -26,6 +26,7 @@ enum rseq_cs_flags_bit { RSEQ_CS_FLAG_NO_RESTART_ON_PREEMPT_BIT = 0, RSEQ_CS_FLAG_NO_RESTART_ON_SIGNAL_BIT = 1, RSEQ_CS_FLAG_NO_RESTART_ON_MIGRATE_BIT = 2, + RSEQ_CS_FLAG_IN_SIGNAL_HANDLER_BIT = 3, }; enum rseq_cs_flags { @@ -35,6 +36,8 @@ enum rseq_cs_flags { (1U << RSEQ_CS_FLAG_NO_RESTART_ON_SIGNAL_BIT), RSEQ_CS_FLAG_NO_RESTART_ON_MIGRATE = (1U << RSEQ_CS_FLAG_NO_RESTART_ON_MIGRATE_BIT), + RSEQ_CS_FLAG_IN_SIGNAL_HANDLER = + (1U << RSEQ_CS_FLAG_IN_SIGNAL_HANDLER_BIT), }; /* @@ -115,7 +118,7 @@ struct rseq { * Restartable sequences flags field. * * This field should only be updated by the thread which - * registered this data structure. Read by the kernel. + * registered this data structure. Read and set by the kernel. * Mainly used for single-stepping through rseq critical sections * with debuggers. * @@ -128,6 +131,9 @@ struct rseq { * - RSEQ_CS_FLAG_NO_RESTART_ON_MIGRATE * Inhibit instruction sequence block restart on migration for * this thread. + * - RSEQ_CS_FLAG_IN_SIGNAL_HANDLER + * Mark that this thread is in critical section but currently in + * signal handlers, so don't clear the rseq_cs field. */ __u32 flags; } __attribute__((aligned(4 * sizeof(__u64)))); diff --git a/kernel/rseq.c b/kernel/rseq.c index 97ac20b4f738..a4aca612724b 100644 --- a/kernel/rseq.c +++ b/kernel/rseq.c @@ -172,14 +172,16 @@ static int rseq_get_rseq_cs(struct task_struct *t, struct rseq_cs *rseq_cs) static int rseq_need_restart(struct task_struct *t, u32 cs_flags) { - u32 flags, event_mask; + u32 old_flags, flags, event_mask; int ret; /* Get thread flags. */ - ret = get_user(flags, &t->rseq->flags); + ret = get_user(old_flags, &t->rseq->flags); if (ret) return ret; + flags = old_flags; + /* Take critical section flags into account. */ flags |= cs_flags; @@ -203,6 +205,21 @@ static int rseq_need_restart(struct task_struct *t, u32 cs_flags) t->rseq_event_mask = 0; preempt_enable(); + /* + * If we don't restart on signal event, set the + * RSEQ_CS_FLAG_IN_SIGNAL_HANDLER flag so that later rseq_ip_fixup + * doesn't clear rseq_cs pointer as the IP is outside of critical + * section. + */ + if ((flags & RSEQ_CS_FLAG_NO_RESTART_ON_SIGNAL) && + (event_mask & RSEQ_EVENT_SIGNAL) && + !(old_flags & RSEQ_CS_FLAG_IN_SIGNAL_HANDLER)) { + old_flags |= RSEQ_CS_FLAG_IN_SIGNAL_HANDLER; + ret = put_user(old_flags, &t->rseq->flags); + if (ret) + return ret; + } + return !!(event_mask & ~flags); } @@ -248,10 +265,23 @@ static int rseq_ip_fixup(struct pt_regs *regs) /* * Handle potentially not being within a critical section. * If not nested over a rseq critical section, restart is useless. - * Clear the rseq_cs pointer and return. + * In case the RSEQ_CS_FLAG_IN_SIGNAL_HANDLER is set, we are in + * signal handlers and later return to critical section so don't + * clear rseq_cs pointer. + * Otherwise, clear the rseq_cs pointer and return. */ - if (!in_rseq_cs(ip, &rseq_cs)) - return clear_rseq_cs(t); + if (!in_rseq_cs(ip, &rseq_cs)) { + u32 flags; + + ret = get_user(flags, &t->rseq->flags); + if (ret) + return ret; + if (flags & RSEQ_CS_FLAG_IN_SIGNAL_HANDLER) + return 0; + else + return clear_rseq_cs(t); + } + ret = rseq_need_restart(t, rseq_cs.flags); if (ret <= 0) return ret; @@ -322,6 +352,46 @@ void rseq_syscall(struct pt_regs *regs) #endif +#ifdef CONFIG_RSEQ + +int rseq_sigreturn(struct pt_regs *regs) +{ + int ret; + struct rseq_cs rseq_cs; + struct task_struct *t = current; + + if (t->rseq) { + u32 flags; + + ret = get_user(flags, &t->rseq->flags); + if (ret) + return ret; + + if (flags & RSEQ_CS_FLAG_IN_SIGNAL_HANDLER) { + ret = rseq_get_rseq_cs(t, &rseq_cs); + if (ret) + return ret; + + /* + * If the returned IP is in critical section, it + * means we handle all the possible nested signal, + * so unset the RSEQ_CS_FLAG_IN_SIGNAL_HANDLER. + */ + if (in_rseq_cs(regs->ip, &rseq_cs)) { + flags &= ~RSEQ_CS_FLAG_IN_SIGNAL_HANDLER; + + ret = put_user(flags, &t->rseq->flags); + if (ret) + return ret; + } + } + } + + return 0; +} + +#endif + /* * sys_rseq - setup restartable sequences for caller thread. */ -- 2.25.1
Powered by blists - more mailing lists