NACK/Cmnt: [PATCH 8/8] x86/bhi: Add support for clearing branch history at syscall entry

[Manuel Diewald]

On Tue, Aug 06, 2024 at 06:50:33PM -0400, Yuxuan Luo wrote:
> (cherry picked from commit 7390db8aea0d64e9deb28b8e1ce716f5020c7ee5)

This is technically a backport, not a cherry pick.

> [yuxuan.luo:
> Backporting this commit again so that the fixes for CVE-2024-25744 will
> not make Jammy vulnerable to CVE-2024-2201 Native BHI again.
> ]
> CVE-2024-25744
> Signed-off-by: Yuxuan Luo <yuxuan.luo at canonical.com>
> ---
>  arch/x86/entry/common.c              |  4 ++--
>  arch/x86/entry/entry_64_compat.S     | 16 ++++++++++++++++
>  arch/x86/include/asm/nospec-branch.h |  4 ++++
>  arch/x86/include/asm/syscall.h       |  1 +
>  4 files changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S
> index 118df23c28d45..f5d0744241ce8 100644
> --- a/arch/x86/entry/entry_64_compat.S
> +++ b/arch/x86/entry/entry_64_compat.S
> @@ -114,6 +114,8 @@ SYM_INNER_LABEL(entry_SYSENTER_compat_after_hwframe, SYM_L_GLOBAL)
>  
>  	cld
>  
> +	CLEAR_BRANCH_HISTORY
> +

I think by adding CLEAR_BRANCH_HISTORY here we re-introduce the issue
that was fixed with

ac8b270b61d4 x86/bhi: Avoid warning in #DB handler due to BHI mitigation

which we was applied to jammy:linux via upstream stable update
v5.15.163 (https://bugs.launchpad.net/bugs/2075170):

cfdff9bb8cca x86/bhi: Avoid warning in #DB handler due to BHI mitigation

It's a fix commit for 7390db8aea0d64e9deb28b8e1ce716f5020c7ee5. By
calling CLEAR_BRANCH_HISTORY too early we will trigger the #DB handler
again. Also, we now call CLEAR_BRANCH_HISTORY twice in
entry_SYSENTER_compat_after_hwframe. I think we need to remove this line
again from entry_64_compat.S.

> diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
> index de97843bdeb80..da464ccdf2269 100644
> --- a/arch/x86/include/asm/nospec-branch.h
> +++ b/arch/x86/include/asm/nospec-branch.h
> @@ -241,6 +241,10 @@ extern void srso_alias_untrain_ret(void);
>  extern void entry_untrain_ret(void);
>  extern void entry_ibpb(void);
>  
> +#ifdef CONFIG_X86_64
> +extern void clear_bhb_loop(void);
> +#endif
> +

This duplicates the existing definition of clear_bhb_loop(void) and is
hence superfluous. We should probably also remove this.

-- 
 Manuel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20240815/bdc22bf7/attachment.sig>