Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Thomas Gleixner <tglx@linutronix.de>
To:     Lai Jiangshan <jiangshanlai@gmail.com>,
        linux-kernel@vger.kernel.org
Cc:     Lai Jiangshan <laijs@linux.alibaba.com>,
        Josh Poimboeuf <jpoimboe@redhat.com>,
        "Chang S . Bae" <chang.seok.bae@intel.com>,
        Sasha Levin <sashal@kernel.org>,
        Andy Lutomirski <luto@kernel.org>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>
Subject: Re: [PATCH V2 01/41] x86/entry: Fix swapgs fence
In-Reply-To: <20210926150838.197719-2-jiangshanlai@gmail.com>
References: <20210926150838.197719-1-jiangshanlai@gmail.com>
 <20210926150838.197719-2-jiangshanlai@gmail.com>
Date:   Sun, 26 Sep 2021 22:43:37 +0200
Message-ID: <87r1dbawzq.ffs@tglx>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

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