linux-kernel - Re: [patch 20/24] x86/speculation: Split out TIF update

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <alpine.DEB.2.21.1811222352340.1665@nanos.tec.linutronix.de>
Date:   Fri, 23 Nov 2018 00:00:55 +0100 (CET)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Tim Chen <tim.c.chen@...ux.intel.com>
cc:     LKML <linux-kernel@...r.kernel.org>, x86@...nel.org,
        Peter Zijlstra <peterz@...radead.org>,
        Andy Lutomirski <luto@...nel.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Jiri Kosina <jkosina@...e.cz>,
        Tom Lendacky <thomas.lendacky@....com>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Andrea Arcangeli <aarcange@...hat.com>,
        David Woodhouse <dwmw@...zon.co.uk>,
        Andi Kleen <ak@...ux.intel.com>,
        Dave Hansen <dave.hansen@...el.com>,
        Casey Schaufler <casey.schaufler@...el.com>,
        Asit Mallick <asit.k.mallick@...el.com>,
        Arjan van de Ven <arjan@...ux.intel.com>,
        Jon Masters <jcm@...hat.com>,
        Waiman Long <longman9394@...il.com>,
        Greg KH <gregkh@...uxfoundation.org>,
        Dave Stewart <david.c.stewart@...el.com>,
        Kees Cook <keescook@...omium.org>
Subject: Re: [patch 20/24] x86/speculation: Split out TIF update

On Wed, 21 Nov 2018, Tim Chen wrote:

> On Wed, Nov 21, 2018 at 09:14:50PM +0100, Thomas Gleixner wrote:
> > +static void task_update_spec_tif(struct task_struct *tsk, int tifbit, bool on)
> >  {
> >       bool update;
> >
> > +     if (on)
> > +             update = !test_and_set_tsk_thread_flag(tsk, tifbit);
> > +     else
> > +             update = test_and_clear_tsk_thread_flag(tsk, tifbit);
> > +
> > +     /*
> > +      * If being set on non-current task, delay setting the CPU
> > +	 * mitigation until it is scheduled next.
> > +	 */
> > +     if (tsk == current && update)
> > +             speculation_ctrl_update_current();
> 
> I think all the call paths from prctl and seccomp coming here
> has tsk == current.

We had that discussion before with SSBD:

seccomp_set_mode_filter()
   seccomp_attach_filter()
      seccomp_sync_threads()
         for_each_thread(t)
	    if (t == current)
              continue;
	    seccomp_assign_mode(t)
	      arch_seccomp_spec_mitigate(t);

seccomp_assign_mode(current...)
  arch_seccomp_spec_mitigate();

> But if task_update_spec_tif gets used in the future where tsk is running
> on a remote CPU, this could lead to the MSR getting out of sync with the
> running task's TIF flag. This will break either performance or security.

We also had that discussion with SSBD and decided that we won't chase
threads and send IPIs around. Yes, it's not perfect, but not the end of the
world either. For PRCTL it's a non issue.

Thanks,

	tglx