linux-kernel - Re: [PATCH 1/3] x86/membarrier: Get rid of a dubious optimization

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CALCETrWYkOp1QkCtzR2APW2KUJV5CGbfae-pTODt76OOB+Mg-A@mail.gmail.com>
Date:   Tue, 1 Dec 2020 09:47:35 -0800
From:   Andy Lutomirski <luto@...nel.org>
To:     Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
Cc:     Andy Lutomirski <luto@...nel.org>, x86 <x86@...nel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Nicholas Piggin <npiggin@...il.com>,
        Arnd Bergmann <arnd@...db.de>,
        Anton Blanchard <anton@...abs.org>
Subject: Re: [PATCH 1/3] x86/membarrier: Get rid of a dubious optimization

On Tue, Dec 1, 2020 at 6:39 AM Mathieu Desnoyers
<mathieu.desnoyers@...icios.com> wrote:
>
> ----- On Nov 30, 2020, at 12:50 PM, Andy Lutomirski luto@...nel.org wrote:
> [...]
> > diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
> > index 11666ba19b62..dabe683ab076 100644
> > --- a/arch/x86/mm/tlb.c
> > +++ b/arch/x86/mm/tlb.c
> > @@ -474,8 +474,10 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct
> > mm_struct *next,
> >       /*
> >        * The membarrier system call requires a full memory barrier and
> >        * core serialization before returning to user-space, after
> > -      * storing to rq->curr. Writing to CR3 provides that full
> > -      * memory barrier and core serializing instruction.
> > +      * storing to rq->curr, when changing mm.  This is because membarrier()
> > +      * sends IPIs to all CPUs that are in the target mm, but another
> > +      * CPU switch to the target mm in the mean time.
>
> The sentence "This is because membarrier() sends IPIs to all CPUs that are in
> the target mm, but another CPU switch to the target mm in the mean time." seems
> rather unclear. Could be clarified with e.g.:
>
> "This is because membarrier() sends IPIs to all CPUs that are in the target mm
> to make them issue memory barriers. However, if another CPU switches to/from the
> target mm concurrently with membarrier(), it can cause that CPU not to receive the
> IPI when it really should issue a memory barrier."
>
> For the rest of this patch:
>
> Reviewed-by: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
>
> Thanks!

Done, thanks!

>
> Mathieu
>
>
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com