linux-kernel - Re: [PATCH V2 01/41] x86/entry: Fix swapgs fence

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]

Date:   Sun, 26 Sep 2021 22:43:37 +0200
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Lai Jiangshan <jiangshanlai@...il.com>,
        linux-kernel@...r.kernel.org
Cc:     Lai Jiangshan <laijs@...ux.alibaba.com>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        "Chang S . Bae" <chang.seok.bae@...el.com>,
        Sasha Levin <sashal@...nel.org>,
        Andy Lutomirski <luto@...nel.org>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        x86@...nel.org, "H. Peter Anvin" <hpa@...or.com>
Subject: Re: [PATCH V2 01/41] x86/entry: Fix swapgs fence

Lai,

On Sun, Sep 26 2021 at 23:07, Lai Jiangshan wrote:
> --- a/arch/x86/entry/entry_64.S
> +++ b/arch/x86/entry/entry_64.S
> @@ -898,17 +898,12 @@ SYM_CODE_START_LOCAL(paranoid_entry)
>  	rdmsr
>  	testl	%edx, %edx
>  	jns	.Lparanoid_entry_swapgs
> +	FENCE_SWAPGS_KERNEL_ENTRY

Good catch.

>  	ret
>  
>  .Lparanoid_entry_swapgs:
>  	swapgs
> -
> -	/*
> -	 * The above SAVE_AND_SWITCH_TO_KERNEL_CR3 macro doesn't do an
> -	 * unconditional CR3 write, even in the PTI case.  So do an lfence
> -	 * to prevent GS speculation, regardless of whether PTI is enabled.
> -	 */
> -	FENCE_SWAPGS_KERNEL_ENTRY
> +	FENCE_SWAPGS_USER_ENTRY

This change is wrong.

In the paranoid entry path even if user GS base is set then the entry
does not necessarily come from user space so there is no guarantee that
there was a CR3 write on PTI enabled systems before the SWAPGS.

FENCE_SWAPGS_USER_ENTRY does not emit a LFENCE when PTI is enabled, so
both the comment and FENCE_SWAPGS_KERNEL_ENTRY which emits LFENCE on
affected CPUs unconditionaly are correct. Though the comment could do
with some polishing to make this entirely clear.

Before adding support for FSGSBASE both the swapgs and non swapgs case
issued the LFENCE unconditionally with FENCE_SWAPGS_KERNEL_ENTRY. The
commit you identified splitted the code pathes and failed to add the
FENCE_SWAPGS_KERNEL_ENTRY into the non-swapgs path.

Thanks,

        tglx