All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rik van Riel <riel@surriel.com>
To: Andy Lutomirski <luto@kernel.org>, x86@kernel.org
Cc: Borislav Petkov <bp@alien8.de>, Jann Horn <jannh@google.com>,
	LKML <linux-kernel@vger.kernel.org>,
	stable@vger.kernel.org, Peter Zijlstra <peterz@infradead.org>,
	Nadav Amit <nadav.amit@gmail.com>
Subject: Re: [PATCH] x86/nmi: Fix some races in NMI uaccess
Date: Mon, 27 Aug 2018 21:31:37 -0400	[thread overview]
Message-ID: <7e33326ecf792c1a2297d162735b3e63ac185ad0.camel@surriel.com> (raw)
In-Reply-To: <c7ee1ee7a1b8d17743f7c3680ea5d2c32bf3e897.1535410956.git.luto@kernel.org>

[-- Attachment #1: Type: text/plain, Size: 1028 bytes --]

On Mon, 2018-08-27 at 16:04 -0700, Andy Lutomirski wrote:

> +++ b/arch/x86/mm/tlb.c
> @@ -345,6 +345,9 @@ void switch_mm_irqs_off(struct mm_struct *prev,
> struct mm_struct *next,
>  		 */
>  		trace_tlb_flush_rcuidle(TLB_FLUSH_ON_TASK_SWITCH,
> TLB_FLUSH_ALL);
>  	} else {
> +		/* Let NMI code know that CR3 may not match
> expectations. */

I don't get it. This is in the "ASID is up to date, do not
need a TLB flush" path.

In what case do we have a TLB that is fully up to date, but
a CR3 that does not match expectations?

Doesn't the CR3 check in nmi_uaccess_ok already catch the
window of time where the CR3 has already been switched over
to that of the next task?

What is special about this path wrt nmi_uaccess_ok that is
not also true for the need_flush branch right above it?

What am I missing?

> +		this_cpu_write(cpu_tlbstate.loaded_mm, NULL);
> +
>  		/* The new ASID is already up to date. */
>  		load_new_mm_cr3(next->pgd, new_asid, false);


 
-- 
All Rights Reversed.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

  parent reply	other threads:[~2018-08-28  1:31 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-27 23:04 [PATCH] x86/nmi: Fix some races in NMI uaccess Andy Lutomirski
2018-08-27 23:12 ` Jann Horn
2018-08-27 23:25   ` Andy Lutomirski
2018-08-27 23:34     ` Jann Horn
2018-08-28  1:31 ` Rik van Riel [this message]
2018-08-28  2:10   ` Andy Lutomirski
2018-08-28 13:50     ` Rik van Riel
2018-08-28 17:56 ` [PATCH v2] " Rik van Riel
2018-08-29  3:46   ` Andy Lutomirski
2018-08-29 15:17     ` Rik van Riel
2018-08-29 15:36       ` Andy Lutomirski
2018-08-29 15:49         ` Rik van Riel
2018-08-29 16:14           ` Andy Lutomirski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7e33326ecf792c1a2297d162735b3e63ac185ad0.camel@surriel.com \
    --to=riel@surriel.com \
    --cc=bp@alien8.de \
    --cc=jannh@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=nadav.amit@gmail.com \
    --cc=peterz@infradead.org \
    --cc=stable@vger.kernel.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.