Message-ID: <CA+i-1C38hxnCGC=Zr=hNFzJBceYoOHfixhpL3xiXEg3hcdgWUg@mail.gmail.com>
Date: Wed, 13 Nov 2024 15:15:43 +0100
From: Brendan Jackman <jackmanb@...gle.com>
To: David Kaplan <david.kaplan@....com>
Cc: Thomas Gleixner <tglx@...utronix.de>, Borislav Petkov <bp@...en8.de>, 
	Peter Zijlstra <peterz@...radead.org>, Josh Poimboeuf <jpoimboe@...nel.org>, 
	Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>, Ingo Molnar <mingo@...hat.com>, 
	Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org, 
	"H . Peter Anvin" <hpa@...or.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 19/35] Documentation/x86: Document the new attack
 vector controls

Hi David,

I'll respond separately to the more interesting thread of discussion
w/ Boris & Derek but here are some more specific comments:

On Tue, 5 Nov 2024 at 23:00, David Kaplan <david.kaplan@....com> wrote:
> +User-to-User
Sorry for the annoying bikeshedding, but this naming could be
misconstrued by a hasty reader as being concerned with the boundary
between Unix users. I wonder if we should say "userspace-to-userspace"
or "process-to-process" or something? The latter would be confusing
because it doesn't imply any exclusion of KVM guests though.

At first I thought "don't be so pedantic, users really need to RTFM
here regardless,
at most we just need a 'note this has nothing to do with Unix users'"

But... actually isn't it conceivable that one day we could want
"mitigate attacks between userspace processes, unless they have the
same effective UID" or something? So then the naming would be really
confusing.

On the other hand, your response to my "but my trust domains are way
more complex than that" comment at LPC was "Google aren't the target
audience for this mechanism". Maybe anyone who knows that their unix
users are meaningful trust domains (probably: someone building a
specialised distro (Android?)) is similarly outside the target
audience here.

> +The user-to-user attack vector involves a malicious userspace program attempting
> +to influence the behavior of another unsuspecting userspace program in order to
> +exfiltrate data.  The vulnerability of a userspace program is based on the
> +program itself and the interfaces it provides.

I find this confusing. "Influence the behaviour" sounds like it's
talking specifically about attacks that drive mis-speculation
(Spectre-type), but shouldn't this also include stuff more in the vein
of L1TF and MDS? I also don't really understand "the interface it
provides".

To be concrete, imagine there's a system where process A just sits in
a loop reading a secret, and on a system with mitigations=off there's
a vuln if a userspace attacker can preempt process A, it can leak the
secret from some uarch buffer it had naturally got into. I expect
mitigate_user_user=on to prevent that, but this wording doesn't really
sound like it does. I guess the vulnerability is "based on the program
itself" in that it took advantage of the fact it read the data into
the buffer, but there's no "interface" and no "influenced the
behaviour".

I think the best I can come up with to describe what I expect this
flag to mitigate is: "a malicious userspace program attempting to leak
data directly from the address space of the victim program". It's a
bit unfortunate that "directly from the address space" implies there's
some other avenue to leak that data, which might not always be the
case and kinda gets back to the other thread about the
user-to-user/user-to-kernel split. Maybe this is a point against that
split.

> +The guest-to-guest attack vector involves a malicious VM [...]
(Ditto of course)

> +Summary of attack-vector mitigations
> +------------------------------------
> +
> +When a vulnerability is mitigated due to an attack-vector control, the default
> +mitigation option for that particular vulnerability is used.  To use a different
> +mitigation, please use the vulnerability-specific command line option.
> +
> +The table below summarizes which vulnerabilities are mitigated when different
> +attack vectors are enabled and assuming the CPU is vulnerable.
> +
> +=============== ============== ============ ============= ============== ============
> +Vulnerability   User-to-Kernel User-to-User Guest-to-Host Guest-to-Guest Cross-Thread
> +=============== ============== ============ ============= ============== ============
> +BHI                   X                           X
> +GDS                   X              X            X              X            X
> +L1TF                                              X                       (Note 1)
> +MDS                   X              X            X              X        (Note 1)
> +MMIO                  X              X            X              X        (Note 1)
> +Meltdown              X
> +Retbleed              X                           X                       (Note 2)
> +RFDS                  X              X            X              X
> +Spectre_v1            X
> +Spectre_v2            X                           X
> +Spectre_v2_user                      X                           X
> +SRBDS                 X              X            X              X
> +SRSO                  X                           X
> +SSB (Note 3)
> +TAA                   X              X            X              X        (Note 1)
> +=============== ============== ============ ============= ============== ============

Hmm, I'm also confused by this. This mechanism is supposed to be about
mitigating attack vectors, but now suddenly we've gone back to talking
about vulnerabilities. Vulns and vectors are obviously not totally
orthogonal but there isn't a 1:1 mapping.

To be concrete again: this seems to say if mitigate_guest_host=off
there's no L1TF mitigation? (I think in fact it's mitigated by PTE
inversion).

And I don't understand why Retbleed doesn't require user_user or
guest_guest mitigation. I assume I can figure this out by reading
about the details of Retbleed exploitation or looking in bugs.c. But
given this is about making things easier I think we should probably
assume the reader is as ignorant about that as I am.

I dunno exactly what to suggest here, but maybe instead of "we
do/don't mitigate these vulns" this would be more helpful as "we
do/don't enable this specific mitigation action". So instead of a row
for L1TF, there would be a row for the flush that l1tf=flush gives you
(basically, like this kinda already has for spectre_v2_user). Maybe
again I'm just being pedantic and making life difficult here though.

Powered by blists - more mailing lists