1. Patchew
  2. linux
  3. [PATCH 0/2] x86/ibt: Fix some spurious ENDBR instances
  4. View patch

[PATCH 2/2] x86/ibt: Avoid duplicate ENDBR in __put_user_nocheck*()

Peter Zijlstra posted 2 patches 2 years, 4 months ago
[PATCH 2/2] x86/ibt: Avoid duplicate ENDBR in __put_user_nocheck*()
Posted by Peter Zijlstra 2 years, 4 months ago 
Commit cb855971d717 ("x86/putuser: Provide room for padding") changed
__put_user_nocheck_*() into proper functions but failed to note that
SYM_FUNC_START() already provides ENDBR, rendering the explicit ENDBR
superfluous.

Fixes: cb855971d717 ("x86/putuser: Provide room for padding")
Reported-by: David Kaplan <David.Kaplan@amd.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/lib/putuser.S |    4 ----
 1 file changed, 4 deletions(-)

--- a/arch/x86/lib/putuser.S
+++ b/arch/x86/lib/putuser.S
@@ -56,7 +56,6 @@ SYM_FUNC_END(__put_user_1)
 EXPORT_SYMBOL(__put_user_1)
 
 SYM_FUNC_START(__put_user_nocheck_1)
-	ENDBR
 	ASM_STAC
 2:	movb %al,(%_ASM_CX)
 	xor %ecx,%ecx
@@ -76,7 +75,6 @@ SYM_FUNC_END(__put_user_2)
 EXPORT_SYMBOL(__put_user_2)
 
 SYM_FUNC_START(__put_user_nocheck_2)
-	ENDBR
 	ASM_STAC
 4:	movw %ax,(%_ASM_CX)
 	xor %ecx,%ecx
@@ -96,7 +94,6 @@ SYM_FUNC_END(__put_user_4)
 EXPORT_SYMBOL(__put_user_4)
 
 SYM_FUNC_START(__put_user_nocheck_4)
-	ENDBR
 	ASM_STAC
 6:	movl %eax,(%_ASM_CX)
 	xor %ecx,%ecx
@@ -119,7 +116,6 @@ SYM_FUNC_END(__put_user_8)
 EXPORT_SYMBOL(__put_user_8)
 
 SYM_FUNC_START(__put_user_nocheck_8)
-	ENDBR
 	ASM_STAC
 9:	mov %_ASM_AX,(%_ASM_CX)
 #ifdef CONFIG_X86_32
Re: [PATCH 2/2] x86/ibt: Avoid duplicate ENDBR in __put_user_nocheck*()
Posted by Peter Zijlstra 2 years, 4 months ago 
On Wed, Aug 02, 2023 at 12:55:47PM +0200, Peter Zijlstra wrote:
> Commit cb855971d717 ("x86/putuser: Provide room for padding") changed
> __put_user_nocheck_*() into proper functions but failed to note that
> SYM_FUNC_START() already provides ENDBR, rendering the explicit ENDBR
> superfluous.

The below objtool thing reliably detects this pattern:

vmlinux.o: warning: objtool: __put_user_nocheck_1+0x4: duplicate ENDBR
vmlinux.o: warning: objtool: __put_user_nocheck_2+0x4: duplicate ENDBR
vmlinux.o: warning: objtool: __put_user_nocheck_4+0x4: duplicate ENDBR
vmlinux.o: warning: objtool: __put_user_nocheck_8+0x4: duplicate ENDBR

---
 tools/objtool/check.c | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 8936a05f0e5a..c6ba6e768b03 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -4254,13 +4254,30 @@ static bool noendbr_range(struct objtool_file *file, struct instruction *insn)
 	return insn->offset == sym->offset + sym->len;
 }
 
-static int validate_ibt_insn(struct objtool_file *file, struct instruction *insn)
+static int validate_ibt_insn(struct objtool_file *file,
+			     struct instruction *insn,
+			     struct instruction *prev)
 {
 	struct instruction *dest;
 	struct reloc *reloc;
 	unsigned long off;
 	int warnings = 0;
 
+	if (prev && prev->sym && prev->sym == insn->sym &&
+	    prev->type == INSN_ENDBR && insn->type == INSN_ENDBR) {
+		/*
+		 * ___bpf_prog_run() is a maze of 'label as value' and
+		 * 'computed goto' and GCC gets confused and emits back-to-back
+		 * ENDBR due to failing to merge two labels.
+		 *
+		 * Don't warn about this until GCC is fixed or verbose.
+		 */
+		if (strcmp(insn->sym->name, "___bpf_prog_run") != 0 || opts.verbose) {
+			WARN_INSN(insn, "duplicate ENDBR");
+			warnings++;
+		}
+	}
+
 	/*
 	 * Looking for function pointer load relocations.  Ignore
 	 * direct/indirect branches:
@@ -4382,11 +4399,13 @@ static int validate_ibt(struct objtool_file *file)
 {
 	struct section *sec;
 	struct reloc *reloc;
-	struct instruction *insn;
+	struct instruction *insn, *prev = NULL;
 	int warnings = 0;
 
-	for_each_insn(file, insn)
-		warnings += validate_ibt_insn(file, insn);
+	for_each_insn(file, insn) {
+		warnings += validate_ibt_insn(file, insn, prev);
+		prev = insn;
+	}
 
 	for_each_sec(file, sec) {
[tip: x86/urgent] x86/ibt: Avoid duplicate ENDBR in __put_user_nocheck*()
Posted by tip-bot2 for Peter Zijlstra 2 years, 3 months ago 
The following commit has been merged into the x86/urgent branch of tip:

Commit-ID:     7575e5a35267983dcbeb1e0d3a49d21ae3cf0b82
Gitweb:        https://git.kernel.org/tip/7575e5a35267983dcbeb1e0d3a49d21ae3cf0b82
Author:        Peter Zijlstra <peterz@infradead.org>
AuthorDate:    Wed, 02 Aug 2023 12:55:47 +02:00
Committer:     Ingo Molnar <mingo@kernel.org>
CommitterDate: Tue, 12 Sep 2023 17:51:24 +02:00

x86/ibt: Avoid duplicate ENDBR in __put_user_nocheck*()

Commit cb855971d717 ("x86/putuser: Provide room for padding") changed
__put_user_nocheck_*() into proper functions but failed to note that
SYM_FUNC_START() already provides ENDBR, rendering the explicit ENDBR
superfluous.

Fixes: cb855971d717 ("x86/putuser: Provide room for padding")
Reported-by: David Kaplan <David.Kaplan@amd.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Link: https://lore.kernel.org/r/20230802110323.086971726@infradead.org
---
 arch/x86/lib/putuser.S | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/arch/x86/lib/putuser.S b/arch/x86/lib/putuser.S
index 1451e0c..235bbda 100644
--- a/arch/x86/lib/putuser.S
+++ b/arch/x86/lib/putuser.S
@@ -56,7 +56,6 @@ SYM_FUNC_END(__put_user_1)
 EXPORT_SYMBOL(__put_user_1)
 
 SYM_FUNC_START(__put_user_nocheck_1)
-	ENDBR
 	ASM_STAC
 2:	movb %al,(%_ASM_CX)
 	xor %ecx,%ecx
@@ -76,7 +75,6 @@ SYM_FUNC_END(__put_user_2)
 EXPORT_SYMBOL(__put_user_2)
 
 SYM_FUNC_START(__put_user_nocheck_2)
-	ENDBR
 	ASM_STAC
 4:	movw %ax,(%_ASM_CX)
 	xor %ecx,%ecx
@@ -96,7 +94,6 @@ SYM_FUNC_END(__put_user_4)
 EXPORT_SYMBOL(__put_user_4)
 
 SYM_FUNC_START(__put_user_nocheck_4)
-	ENDBR
 	ASM_STAC
 6:	movl %eax,(%_ASM_CX)
 	xor %ecx,%ecx
@@ -119,7 +116,6 @@ SYM_FUNC_END(__put_user_8)
 EXPORT_SYMBOL(__put_user_8)
 
 SYM_FUNC_START(__put_user_nocheck_8)
-	ENDBR
 	ASM_STAC
 9:	mov %_ASM_AX,(%_ASM_CX)
 #ifdef CONFIG_X86_32
[tip: x86/urgent] x86/ibt: Avoid duplicate ENDBR in __put_user_nocheck*()
Posted by tip-bot2 for Peter Zijlstra 2 years, 3 months ago 
The following commit has been merged into the x86/urgent branch of tip:

Commit-ID:     1666863f28d7c490fa62ed5d1dfb1ba31453dc98
Gitweb:        https://git.kernel.org/tip/1666863f28d7c490fa62ed5d1dfb1ba31453dc98
Author:        Peter Zijlstra <peterz@infradead.org>
AuthorDate:    Wed, 02 Aug 2023 12:55:47 +02:00
Committer:     root <root@noisy.programming.kicks-ass.net>
CommitterDate: Tue, 12 Sep 2023 12:40:28 +02:00

x86/ibt: Avoid duplicate ENDBR in __put_user_nocheck*()

Commit cb855971d717 ("x86/putuser: Provide room for padding") changed
__put_user_nocheck_*() into proper functions but failed to note that
SYM_FUNC_START() already provides ENDBR, rendering the explicit ENDBR
superfluous.

Fixes: cb855971d717 ("x86/putuser: Provide room for padding")
Reported-by: David Kaplan <David.Kaplan@amd.com>
Signed-off-by: "Peter Zijlstra (Intel)" <peterz@infradead.org>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lore.kernel.org/r/20230802110323.086971726@infradead.org
---
 arch/x86/lib/putuser.S | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/arch/x86/lib/putuser.S b/arch/x86/lib/putuser.S
index 1451e0c..235bbda 100644
--- a/arch/x86/lib/putuser.S
+++ b/arch/x86/lib/putuser.S
@@ -56,7 +56,6 @@ SYM_FUNC_END(__put_user_1)
 EXPORT_SYMBOL(__put_user_1)
 
 SYM_FUNC_START(__put_user_nocheck_1)
-	ENDBR
 	ASM_STAC
 2:	movb %al,(%_ASM_CX)
 	xor %ecx,%ecx
@@ -76,7 +75,6 @@ SYM_FUNC_END(__put_user_2)
 EXPORT_SYMBOL(__put_user_2)
 
 SYM_FUNC_START(__put_user_nocheck_2)
-	ENDBR
 	ASM_STAC
 4:	movw %ax,(%_ASM_CX)
 	xor %ecx,%ecx
@@ -96,7 +94,6 @@ SYM_FUNC_END(__put_user_4)
 EXPORT_SYMBOL(__put_user_4)
 
 SYM_FUNC_START(__put_user_nocheck_4)
-	ENDBR
 	ASM_STAC
 6:	movl %eax,(%_ASM_CX)
 	xor %ecx,%ecx
@@ -119,7 +116,6 @@ SYM_FUNC_END(__put_user_8)
 EXPORT_SYMBOL(__put_user_8)
 
 SYM_FUNC_START(__put_user_nocheck_8)
-	ENDBR
 	ASM_STAC
 9:	mov %_ASM_AX,(%_ASM_CX)
 #ifdef CONFIG_X86_32

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.