| 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
| ||
|
Message-ID: <1068963628.19295.1506458790412.JavaMail.zimbra@efficios.com>
Date: Tue, 26 Sep 2017 20:46:30 +0000 (UTC)
From: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
To: "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Thomas Gleixner <tglx@...utronix.de>,
"H. Peter Anvin" <hpa@...or.com>
Cc: linux-kernel <linux-kernel@...r.kernel.org>,
Boqun Feng <boqun.feng@...il.com>,
Andrew Hunter <ahh@...gle.com>,
maged michael <maged.michael@...il.com>,
gromer <gromer@...gle.com>, Avi Kivity <avi@...lladb.com>,
Benjamin Herrenschmidt <benh@...nel.crashing.org>,
Paul Mackerras <paulus@...ba.org>,
Michael Ellerman <mpe@...erman.id.au>,
Dave Watson <davejwatson@...com>, x86@...nel.org
Subject: Re: [PATCH v4 for 4.14 3/3] membarrier: Document scheduler barrier
requirements
----- On Sep 26, 2017, at 1:51 PM, Mathieu Desnoyers mathieu.desnoyers@...icios.com wrote:
> Document the membarrier requirement on having a full memory barrier in
> __schedule() after coming from user-space, before storing to rq->curr.
> It is provided by smp_mb__after_spinlock() in __schedule().
Missed a few maintainers that should have been CC'd. Adding them now.
This patch is aimed to go through Paul E. McKenney's tree.
Thanks,
Mathieu
>
> Document that membarrier requires a full barrier on transition from
> kernel thread to userspace thread. We currently have an implicit barrier
> from atomic_dec_and_test() in mmdrop() that ensures this.
>
> The x86 switch_mm_irqs_off() full barrier is currently provided by many
> cpumask update operations as well as write_cr3(). Document that
> write_cr3() provides this barrier.
>
> Changes since v1:
> - Update comments to match reality for code paths which are after
> storing to rq->curr, before returning to user-space.
> Changes since v2:
> - Update changelog (smp_mb__before_spinlock -> smp_mb__after_spinlock).
> Changes since v3:
> - Clarify comments following feeback from Peter Zijlstra.
>
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
> CC: Peter Zijlstra <peterz@...radead.org>
> CC: Paul E. McKenney <paulmck@...ux.vnet.ibm.com>
> CC: Boqun Feng <boqun.feng@...il.com>
> CC: Andrew Hunter <ahh@...gle.com>
> CC: Maged Michael <maged.michael@...il.com>
> CC: gromer@...gle.com
> CC: Avi Kivity <avi@...lladb.com>
> CC: Benjamin Herrenschmidt <benh@...nel.crashing.org>
> CC: Paul Mackerras <paulus@...ba.org>
> CC: Michael Ellerman <mpe@...erman.id.au>
> CC: Dave Watson <davejwatson@...com>
> ---
> arch/x86/mm/tlb.c | 5 +++++
> include/linux/sched/mm.h | 5 +++++
> kernel/sched/core.c | 38 +++++++++++++++++++++++++++-----------
> 3 files changed, 37 insertions(+), 11 deletions(-)
>
> diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
> index 93fe97cce581..5ba86b85953b 100644
> --- a/arch/x86/mm/tlb.c
> +++ b/arch/x86/mm/tlb.c
> @@ -143,6 +143,11 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct
> mm_struct *next,
> }
> #endif
>
> + /*
> + * The membarrier system call requires a full memory barrier
> + * before returning to user-space, after storing to rq->curr.
> + * Writing to CR3 provides that full memory barrier.
> + */
> if (real_prev == next) {
> VM_BUG_ON(this_cpu_read(cpu_tlbstate.ctxs[prev_asid].ctx_id) !=
> next->context.ctx_id);
> diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h
> index d3b81e48784d..1bd10c2c0893 100644
> --- a/include/linux/sched/mm.h
> +++ b/include/linux/sched/mm.h
> @@ -38,6 +38,11 @@ static inline void mmgrab(struct mm_struct *mm)
> extern void __mmdrop(struct mm_struct *);
> static inline void mmdrop(struct mm_struct *mm)
> {
> + /*
> + * The implicit full barrier implied by atomic_dec_and_test is
> + * required by the membarrier system call before returning to
> + * user-space, after storing to rq->curr.
> + */
> if (unlikely(atomic_dec_and_test(&mm->mm_count)))
> __mmdrop(mm);
> }
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index b9d731283946..6254f87645de 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -2649,6 +2649,12 @@ static struct rq *finish_task_switch(struct task_struct
> *prev)
> finish_arch_post_lock_switch();
>
> fire_sched_in_preempt_notifiers(current);
> + /*
> + * When transitioning from a kernel thread to a userspace
> + * thread, mmdrop()'s implicit full barrier is required by the
> + * membarrier system call, because the current active_mm can
> + * become the current mm without going through switch_mm().
> + */
> if (mm)
> mmdrop(mm);
> if (unlikely(prev_state == TASK_DEAD)) {
> @@ -2754,6 +2760,13 @@ context_switch(struct rq *rq, struct task_struct *prev,
> */
> arch_start_context_switch(prev);
>
> + /*
> + * If mm is non-NULL, we pass through switch_mm(). If mm is
> + * NULL, we will pass through mmdrop() in finish_task_switch().
> + * Both of these contain the full memory barrier required by
> + * membarrier after storing to rq->curr, before returning to
> + * user-space.
> + */
> if (!mm) {
> next->active_mm = oldmm;
> mmgrab(oldmm);
> @@ -3290,6 +3303,9 @@ static void __sched notrace __schedule(bool preempt)
> * Make sure that signal_pending_state()->signal_pending() below
> * can't be reordered with __set_current_state(TASK_INTERRUPTIBLE)
> * done by the caller to avoid the race with signal_wake_up().
> + *
> + * The membarrier system call requires a full memory barrier
> + * after coming from user-space, before storing to rq->curr.
> */
> rq_lock(rq, &rf);
> smp_mb__after_spinlock();
> @@ -3337,17 +3353,17 @@ static void __sched notrace __schedule(bool preempt)
> /*
> * The membarrier system call requires each architecture
> * to have a full memory barrier after updating
> - * rq->curr, before returning to user-space. For TSO
> - * (e.g. x86), the architecture must provide its own
> - * barrier in switch_mm(). For weakly ordered machines
> - * for which spin_unlock() acts as a full memory
> - * barrier, finish_lock_switch() in common code takes
> - * care of this barrier. For weakly ordered machines for
> - * which spin_unlock() acts as a RELEASE barrier (only
> - * arm64 and PowerPC), arm64 has a full barrier in
> - * switch_to(), and PowerPC has
> - * smp_mb__after_unlock_lock() before
> - * finish_lock_switch().
> + * rq->curr, before returning to user-space.
> + *
> + * Here are the schemes providing that barrier on the
> + * various architectures:
> + * - mm ? switch_mm() : mmdrop() for x86, s390, sparc,
> + * - finish_lock_switch() for weakly-ordered
> + * architectures where spin_unlock is a full barrier,
> + * - switch_to() for arm64 (weakly-ordered, spin_unlock
> + * is a RELEASE barrier),
> + * - membarrier_arch_sched_in() for PowerPC,
> + * (weakly-ordered, spin_unlock is a RELEASE barrier).
> */
> ++*switch_count;
>
> --
> 2.11.0
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
Powered by blists - more mailing lists