linux-kernel - RE: [PATCH v3 21/35] x86/bugs: Determine relevant vulnerabilities based on attack vector controls.

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:
 <LV3PR12MB92652B8AFE3DC6EADE24E7CC94132@LV3PR12MB9265.namprd12.prod.outlook.com>
Date: Thu, 9 Jan 2025 15:08:58 +0000
From: "Kaplan, David" <David.Kaplan@....com>
To: Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>
CC: Thomas Gleixner <tglx@...utronix.de>, Borislav Petkov <bp@...en8.de>,
	Peter Zijlstra <peterz@...radead.org>, Josh Poimboeuf <jpoimboe@...nel.org>,
	Ingo Molnar <mingo@...hat.com>, Dave Hansen <dave.hansen@...ux.intel.com>,
	"x86@...nel.org" <x86@...nel.org>, "H . Peter Anvin" <hpa@...or.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v3 21/35] x86/bugs: Determine relevant vulnerabilities
 based on attack vector controls.

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>
> Sent: Wednesday, January 8, 2025 9:43 PM
> To: Kaplan, David <David.Kaplan@....com>
> Cc: Thomas Gleixner <tglx@...utronix.de>; Borislav Petkov <bp@...en8.de>; Peter
> Zijlstra <peterz@...radead.org>; Josh Poimboeuf <jpoimboe@...nel.org>; 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 v3 21/35] x86/bugs: Determine relevant vulnerabilities based
> on attack vector controls.
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On Wed, Jan 08, 2025 at 02:25:01PM -0600, David Kaplan wrote:
> > The function should_mitigate_vuln() defines which vulnerabilities
> > should be mitigated based on the selected attack vector controls.  The
> > selections here are based on the individual characteristics of each
> > vulnerability.
> >
> > Signed-off-by: David Kaplan <david.kaplan@....com>
> > ---
> >  arch/x86/kernel/cpu/bugs.c | 69
> > ++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 69 insertions(+)
> >
> > diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> > index 88eba8e4c7fb..175dbbf9b06e 100644
> > --- a/arch/x86/kernel/cpu/bugs.c
> > +++ b/arch/x86/kernel/cpu/bugs.c
> > @@ -347,6 +347,75 @@ static void x86_amd_ssb_disable(void)
> >               wrmsrl(MSR_AMD64_LS_CFG, msrval);  }
> >
> > +/*
> > + * Returns true if vulnerability should be mitigated based on the
> > + * selected attack vector controls
> > + *
> > + * See Documentation/admin-guide/hw-vuln/attack_vector_controls.rst
> > + */
> > +static bool __init should_mitigate_vuln(unsigned int bug) {
> > +     switch (bug) {
> > +     /*
> > +      * The only spectre_v1 mitigations in the kernel are related to
> > +      * SWAPGS protection on kernel entry.  Therefore, protection is
> > +      * only required for the user->kernel attack vector.
> > +      */
> > +     case X86_BUG_SPECTRE_V1:
> > +             return
> > +cpu_mitigate_attack_vector(CPU_MITIGATE_USER_KERNEL);
> > +
> > +     /*
> > +      * Both spectre_v2 and srso may allow user->kernel or
> > +      * guest->host attacks through branch predictor manipulation.
> > +      */
> > +     case X86_BUG_SPECTRE_V2:
> > +     case X86_BUG_SRSO:
> > +             return cpu_mitigate_attack_vector(CPU_MITIGATE_USER_KERNEL)
> ||
> > +
> > + cpu_mitigate_attack_vector(CPU_MITIGATE_GUEST_HOST);
> > +
> > +     /*
> > +      * spectre_v2_user refers to user->user or guest->guest branch
> > +      * predictor attacks only.  Other indirect branch predictor attacks
> > +      * are covered by the spectre_v2 vulnerability.
> > +      */
> > +     case X86_BUG_SPECTRE_V2_USER:
> > +             return cpu_mitigate_attack_vector(CPU_MITIGATE_USER_USER) ||
> > +
> > + cpu_mitigate_attack_vector(CPU_MITIGATE_GUEST_GUEST);
> > +
> > +     /* L1TF is only possible as a guest->host attack */
> > +     case X86_BUG_L1TF:
> > +             return
> > + cpu_mitigate_attack_vector(CPU_MITIGATE_GUEST_HOST);
> > +
> > +     /*
> > +      * All the vulnerabilities below allow potentially leaking data
> > +      * across address spaces.  Therefore, mitigation is required for
> > +      * any of these 4 attack vectors.
> > +      */
> > +     case X86_BUG_MDS:
> > +     case X86_BUG_TAA:
> > +     case X86_BUG_MMIO_STALE_DATA:
> > +     case X86_BUG_RFDS:
> > +     case X86_BUG_SRBDS:
> > +             return cpu_mitigate_attack_vector(CPU_MITIGATE_USER_KERNEL)
> ||
> > +                     cpu_mitigate_attack_vector(CPU_MITIGATE_GUEST_HOST) ||
> > +                     cpu_mitigate_attack_vector(CPU_MITIGATE_USER_USER) ||
> > +                     cpu_mitigate_attack_vector(CPU_MITIGATE_GUEST_GUEST);
> > +     /*
> > +      * GDS can potentially leak data across address spaces and
> > +      * threads.  Mitigation is required under all attack vectors.
> > +      */
> > +     case X86_BUG_GDS:
> > +             return cpu_mitigate_attack_vector(CPU_MITIGATE_USER_KERNEL)
> ||
> > +                     cpu_mitigate_attack_vector(CPU_MITIGATE_GUEST_HOST) ||
> > +                     cpu_mitigate_attack_vector(CPU_MITIGATE_USER_USER) ||
> > +                     cpu_mitigate_attack_vector(CPU_MITIGATE_GUEST_GUEST) ||
> > +                     cpu_mitigate_attack_vector(CPU_MITIGATE_CROSS_THREAD);
> > +     default:
> > +             return false;
>
> It is missing the case X86_BUG_RETBLEED. should_mitigate_vuln() will always
> return false for retbleed.

Good catch! Not sure how I missed that but will fix.  It should be in the same group as spectre_v2/srso

>
> I am wondering if this function should return true in the default case. So that in future
> if someone misses to add a case for a new bug, it will still be mitigated.

Perhaps a warning would also be appropriate, which would have made this issue easier to spot.

Thanks
--David Kaplan