Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <63e23f80-033f-f64e-7522-2816debbc367@kernel.org>
Date:   Wed, 13 Jul 2022 09:45:26 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.10.0
Subject: Re: [PATCH 5.18 34/61] objtool: Update Retpoline validation
Content-Language: en-US
To:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        linux-kernel@vger.kernel.org
Cc:     stable@vger.kernel.org,
        "Peter Zijlstra (Intel)" <peterz@infradead.org>,
        Borislav Petkov <bp@suse.de>,
        Josh Poimboeuf <jpoimboe@kernel.org>,
        Thadeu Lima de Souza Cascardo <cascardo@canonical.com>
References: <20220712183236.931648980@linuxfoundation.org>
 <20220712183238.342232911@linuxfoundation.org>
From:   Jiri Slaby <jirislaby@kernel.org>
In-Reply-To: <20220712183238.342232911@linuxfoundation.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 12. 07. 22, 20:39, Greg Kroah-Hartman wrote:
> From: Peter Zijlstra <peterz@infradead.org>
> 
> commit 9bb2ec608a209018080ca262f771e6a9ff203b6f upstream.
> 
> Update retpoline validation with the new CONFIG_RETPOLINE requirement of
> not having bare naked RET instructions.

Hi,

this breaks compilation on i386:
 > arch/x86/kernel/../../x86/xen/xen-head.S:35: Error: no such 
instruction: `annotate_unret_safe'

Config:
https://raw.githubusercontent.com/openSUSE/kernel-source/stable/config/i386/pae

And yeah, upstream¹⁾ is affected too.

¹⁾I am at commit b047602d579b4fb028128a525f056bbdc890e7f0.

> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Signed-off-by: Borislav Petkov <bp@suse.de>
> Reviewed-by: Josh Poimboeuf <jpoimboe@kernel.org>
> Signed-off-by: Borislav Petkov <bp@suse.de>
> Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>   arch/x86/include/asm/nospec-branch.h |    6 ++++++
>   arch/x86/mm/mem_encrypt_boot.S       |    2 ++
>   arch/x86/xen/xen-head.S              |    1 +
>   tools/objtool/check.c                |   19 +++++++++++++------
>   4 files changed, 22 insertions(+), 6 deletions(-)
> 
> --- a/arch/x86/include/asm/nospec-branch.h
> +++ b/arch/x86/include/asm/nospec-branch.h
> @@ -76,6 +76,12 @@
>   .endm
>   
>   /*
> + * (ab)use RETPOLINE_SAFE on RET to annotate away 'bare' RET instructions
> + * vs RETBleed validation.
> + */
> +#define ANNOTATE_UNRET_SAFE ANNOTATE_RETPOLINE_SAFE
> +
> +/*
>    * JMP_NOSPEC and CALL_NOSPEC macros can be used instead of a simple
>    * indirect jmp/call which may be susceptible to the Spectre variant 2
>    * attack.
> --- a/arch/x86/mm/mem_encrypt_boot.S
> +++ b/arch/x86/mm/mem_encrypt_boot.S
> @@ -66,6 +66,7 @@ SYM_FUNC_START(sme_encrypt_execute)
>   	pop	%rbp
>   
>   	/* Offset to __x86_return_thunk would be wrong here */
> +	ANNOTATE_UNRET_SAFE
>   	ret
>   	int3
>   SYM_FUNC_END(sme_encrypt_execute)
> @@ -154,6 +155,7 @@ SYM_FUNC_START(__enc_copy)
>   	pop	%r15
>   
>   	/* Offset to __x86_return_thunk would be wrong here */
> +	ANNOTATE_UNRET_SAFE
>   	ret
>   	int3
>   .L__enc_copy_end:
> --- a/arch/x86/xen/xen-head.S
> +++ b/arch/x86/xen/xen-head.S
> @@ -26,6 +26,7 @@ SYM_CODE_START(hypercall_page)
>   	.rept (PAGE_SIZE / 32)
>   		UNWIND_HINT_FUNC
>   		ANNOTATE_NOENDBR
> +		ANNOTATE_UNRET_SAFE
>   		ret
>   		/*
>   		 * Xen will write the hypercall page, and sort out ENDBR.
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -2114,8 +2114,9 @@ static int read_retpoline_hints(struct o
>   		}
>   
>   		if (insn->type != INSN_JUMP_DYNAMIC &&
> -		    insn->type != INSN_CALL_DYNAMIC) {
> -			WARN_FUNC("retpoline_safe hint not an indirect jump/call",
> +		    insn->type != INSN_CALL_DYNAMIC &&
> +		    insn->type != INSN_RETURN) {
> +			WARN_FUNC("retpoline_safe hint not an indirect jump/call/ret",
>   				  insn->sec, insn->offset);
>   			return -1;
>   		}
> @@ -3648,7 +3649,8 @@ static int validate_retpoline(struct obj
>   
>   	for_each_insn(file, insn) {
>   		if (insn->type != INSN_JUMP_DYNAMIC &&
> -		    insn->type != INSN_CALL_DYNAMIC)
> +		    insn->type != INSN_CALL_DYNAMIC &&
> +		    insn->type != INSN_RETURN)
>   			continue;
>   
>   		if (insn->retpoline_safe)
> @@ -3663,9 +3665,14 @@ static int validate_retpoline(struct obj
>   		if (!strcmp(insn->sec->name, ".init.text") && !module)
>   			continue;
>   
> -		WARN_FUNC("indirect %s found in RETPOLINE build",
> -			  insn->sec, insn->offset,
> -			  insn->type == INSN_JUMP_DYNAMIC ? "jump" : "call");
> +		if (insn->type == INSN_RETURN) {
> +			WARN_FUNC("'naked' return found in RETPOLINE build",
> +				  insn->sec, insn->offset);
> +		} else {
> +			WARN_FUNC("indirect %s found in RETPOLINE build",
> +				  insn->sec, insn->offset,
> +				  insn->type == INSN_JUMP_DYNAMIC ? "jump" : "call");
> +		}
>   
>   		warnings++;
>   	}
> 
> 


-- 
js
suse labs