[v2] objtool: Fix return thunk patching in retpolines

[PATCH v2] objtool: Fix return thunk patching in retpolines

Posted by Josh Poimboeuf 2 years, 2 months ago

With CONFIG_RETHUNK enabled, the compiler replaces every RET with a tail
call to a return thunk ('JMP __x86_return_thunk').  Objtool annotates
all such return sites so they can be patched during boot by
apply_returns().

The implementation of __x86_return_thunk() is just a bare RET.  It's
only meant to be used temporarily until apply_returns() patches all
return sites with either a JMP to another return thunk or an actual RET.

The following commit

  e92626af3234 ("x86/retpoline: Remove .text..__x86.return_thunk section") retpolines

broke objtool's detection of return sites in retpolines.  Since
retpolines and return thunks are now in the same section, the compiler
no longer uses relocations for the intra-section jumps between the
retpolines and the return thunk, causing objtool to overlook them.

As a result, none of the retpolines' return sites get patched.  Each one
stays at 'JMP __x86_return_thunk', effectively a bare RET.

Fix it by teaching objtool to detect when a non-relocated jump target is
a return thunk (or retpoline).

Fixes: e92626af3234 ("x86/retpoline: Remove .text..__x86.return_thunk section")
Reported-by: David Kaplan <david.kaplan@amd.com>
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 tools/objtool/check.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index e308d1ba664e..e94756e09ca9 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1610,6 +1610,22 @@ static int add_jump_destinations(struct objtool_file *file)
 			return -1;
 		}
 
+		/*
+		 * An intra-TU jump in retpoline.o might not have a relocation
+		 * for its jump dest, in which case the above
+		 * add_{retpoline,return}_call() didn't happen.
+		 */
+		if (jump_dest->sym && jump_dest->offset == jump_dest->sym->offset) {
+			if (jump_dest->sym->retpoline_thunk) {
+				add_retpoline_call(file, insn);
+				continue;
+			}
+			if (jump_dest->sym->return_thunk) {
+				add_return_call(file, insn, true);
+				continue;
+			}
+		}
+
 		/*
 		 * Cross-function jump.
 		 */
-- 
2.41.0

Re: [PATCH v2] objtool: Fix return thunk patching in retpolines

Posted by Peter Zijlstra 2 years, 2 months ago

On Wed, Oct 11, 2023 at 07:47:37PM -0700, Josh Poimboeuf wrote:
> With CONFIG_RETHUNK enabled, the compiler replaces every RET with a tail
> call to a return thunk ('JMP __x86_return_thunk').  Objtool annotates
> all such return sites so they can be patched during boot by
> apply_returns().
> 
> The implementation of __x86_return_thunk() is just a bare RET.  It's
> only meant to be used temporarily until apply_returns() patches all
> return sites with either a JMP to another return thunk or an actual RET.
> 
> The following commit
> 
>   e92626af3234 ("x86/retpoline: Remove .text..__x86.return_thunk section") retpolines
> 
> broke objtool's detection of return sites in retpolines.  Since
> retpolines and return thunks are now in the same section, the compiler
> no longer uses relocations for the intra-section jumps between the
> retpolines and the return thunk, causing objtool to overlook them.
> 
> As a result, none of the retpolines' return sites get patched.  Each one
> stays at 'JMP __x86_return_thunk', effectively a bare RET.
> 
> Fix it by teaching objtool to detect when a non-relocated jump target is
> a return thunk (or retpoline).
> 
> Fixes: e92626af3234 ("x86/retpoline: Remove .text..__x86.return_thunk section")
> Reported-by: David Kaplan <david.kaplan@amd.com>
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>

> ---
>  tools/objtool/check.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> index e308d1ba664e..e94756e09ca9 100644
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -1610,6 +1610,22 @@ static int add_jump_destinations(struct objtool_file *file)
>  			return -1;
>  		}
>  
> +		/*
> +		 * An intra-TU jump in retpoline.o might not have a relocation
> +		 * for its jump dest, in which case the above
> +		 * add_{retpoline,return}_call() didn't happen.
> +		 */
> +		if (jump_dest->sym && jump_dest->offset == jump_dest->sym->offset) {
> +			if (jump_dest->sym->retpoline_thunk) {
> +				add_retpoline_call(file, insn);
> +				continue;
> +			}
> +			if (jump_dest->sym->return_thunk) {
> +				add_return_call(file, insn, true);
> +				continue;
> +			}
> +		}
> +
>  		/*
>  		 * Cross-function jump.
>  		 */
> -- 
> 2.41.0
>

[tip: x86/bugs] objtool: Fix return thunk patching in retpolines

Posted by tip-bot2 for Josh Poimboeuf 2 years, 1 month ago

The following commit has been merged into the x86/bugs branch of tip:

Commit-ID:     34de4fe7d1326c5c27890df3297dffd4c7196b0e
Gitweb:        https://git.kernel.org/tip/34de4fe7d1326c5c27890df3297dffd4c7196b0e
Author:        Josh Poimboeuf <jpoimboe@kernel.org>
AuthorDate:    Wed, 11 Oct 2023 19:47:37 -07:00
Committer:     Borislav Petkov (AMD) <bp@alien8.de>
CommitterDate: Fri, 20 Oct 2023 12:51:41 +02:00

objtool: Fix return thunk patching in retpolines

With CONFIG_RETHUNK enabled, the compiler replaces every RET with a tail
call to a return thunk ('JMP __x86_return_thunk').  Objtool annotates
all such return sites so they can be patched during boot by
apply_returns().

The implementation of __x86_return_thunk() is just a bare RET.  It's
only meant to be used temporarily until apply_returns() patches all
return sites with either a JMP to another return thunk or an actual RET.

Removing the .text..__x86.return_thunk section would break objtool's
detection of return sites in retpolines.  Since retpolines and return
thunks would land in the same section, the compiler no longer uses
relocations for the intra-section jumps between the retpolines and the
return thunk, causing objtool to overlook them.

As a result, none of the retpolines' return sites would get patched.
Each one stays at 'JMP __x86_return_thunk', effectively a bare RET.

Fix it by teaching objtool to detect when a non-relocated jump target is
a return thunk (or retpoline).

  [ bp: Massage the commit message now that the offending commit
    removing the .text..__x86.return_thunk section has been zapped.
    Still keep the objtool change here as it makes objtool more robust
    wrt handling such intra-TU jumps without relocations, should some
    toolchain and/or config generate them in the future. ]

Reported-by: David Kaplan <david.kaplan@amd.com>
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lore.kernel.org/r/20231012024737.eg5phclogp67ik6x@treble
---
 tools/objtool/check.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index e308d1b..e94756e 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1611,6 +1611,22 @@ static int add_jump_destinations(struct objtool_file *file)
 		}
 
 		/*
+		 * An intra-TU jump in retpoline.o might not have a relocation
+		 * for its jump dest, in which case the above
+		 * add_{retpoline,return}_call() didn't happen.
+		 */
+		if (jump_dest->sym && jump_dest->offset == jump_dest->sym->offset) {
+			if (jump_dest->sym->retpoline_thunk) {
+				add_retpoline_call(file, insn);
+				continue;
+			}
+			if (jump_dest->sym->return_thunk) {
+				add_return_call(file, insn, true);
+				continue;
+			}
+		}
+
+		/*
 		 * Cross-function jump.
 		 */
 		if (insn_func(insn) && insn_func(jump_dest) &&

[tip: x86/bugs] objtool: Fix return thunk patching in retpolines

Posted by tip-bot2 for Josh Poimboeuf 2 years, 2 months ago

The following commit has been merged into the x86/bugs branch of tip:

Commit-ID:     eadbe6606a85610c63e6cfaa9257dc7e3bbb901c
Gitweb:        https://git.kernel.org/tip/eadbe6606a85610c63e6cfaa9257dc7e3bbb901c
Author:        Josh Poimboeuf <jpoimboe@kernel.org>
AuthorDate:    Wed, 11 Oct 2023 19:47:37 -07:00
Committer:     Ingo Molnar <mingo@kernel.org>
CommitterDate: Thu, 12 Oct 2023 19:43:51 +02:00

objtool: Fix return thunk patching in retpolines

With CONFIG_RETHUNK enabled, the compiler replaces every RET with a tail
call to a return thunk ('JMP __x86_return_thunk').  Objtool annotates
all such return sites so they can be patched during boot by
apply_returns().

The implementation of __x86_return_thunk() is just a bare RET.  It's
only meant to be used temporarily until apply_returns() patches all
return sites with either a JMP to another return thunk or an actual RET.

The following commit:

  e92626af3234 ("x86/retpoline: Remove .text..__x86.return_thunk section")

broke objtool's detection of return sites in retpolines.  Since
retpolines and return thunks are now in the same section, the compiler
no longer uses relocations for the intra-section jumps between the
retpolines and the return thunk, causing objtool to overlook them.

As a result, none of the retpolines' return sites get patched.  Each one
stays at 'JMP __x86_return_thunk', effectively a bare RET.

Fix it by teaching objtool to detect when a non-relocated jump target is
a return thunk (or retpoline).

Fixes: e92626af3234 ("x86/retpoline: Remove .text..__x86.return_thunk section")
Reported-by: David Kaplan <david.kaplan@amd.com>
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lore.kernel.org/r/20231012024737.eg5phclogp67ik6x@treble
---
 tools/objtool/check.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index e308d1b..e94756e 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1611,6 +1611,22 @@ static int add_jump_destinations(struct objtool_file *file)
 		}
 
 		/*
+		 * An intra-TU jump in retpoline.o might not have a relocation
+		 * for its jump dest, in which case the above
+		 * add_{retpoline,return}_call() didn't happen.
+		 */
+		if (jump_dest->sym && jump_dest->offset == jump_dest->sym->offset) {
+			if (jump_dest->sym->retpoline_thunk) {
+				add_retpoline_call(file, insn);
+				continue;
+			}
+			if (jump_dest->sym->return_thunk) {
+				add_return_call(file, insn, true);
+				continue;
+			}
+		}
+
+		/*
 		 * Cross-function jump.
 		 */
 		if (insn_func(insn) && insn_func(jump_dest) &&

[tip: x86/bugs] objtool: Fix return thunk patching in retpolines

Posted by tip-bot2 for Josh Poimboeuf 2 years, 2 months ago

The following commit has been merged into the x86/bugs branch of tip:

Commit-ID:     3ab1bb69862d4477e2ffa556075a251bd7328910
Gitweb:        https://git.kernel.org/tip/3ab1bb69862d4477e2ffa556075a251bd7328910
Author:        Josh Poimboeuf <jpoimboe@kernel.org>
AuthorDate:    Wed, 11 Oct 2023 19:47:37 -07:00
Committer:     Ingo Molnar <mingo@kernel.org>
CommitterDate: Thu, 12 Oct 2023 08:21:29 +02:00

objtool: Fix return thunk patching in retpolines

With CONFIG_RETHUNK enabled, the compiler replaces every RET with a tail
call to a return thunk ('JMP __x86_return_thunk').  Objtool annotates
all such return sites so they can be patched during boot by
apply_returns().

The implementation of __x86_return_thunk() is just a bare RET.  It's
only meant to be used temporarily until apply_returns() patches all
return sites with either a JMP to another return thunk or an actual RET.

The following commit:

  e92626af3234 ("x86/retpoline: Remove .text..__x86.return_thunk section")

broke objtool's detection of return sites in retpolines.  Since
retpolines and return thunks are now in the same section, the compiler
no longer uses relocations for the intra-section jumps between the
retpolines and the return thunk, causing objtool to overlook them.

As a result, none of the retpolines' return sites get patched.  Each one
stays at 'JMP __x86_return_thunk', effectively a bare RET.

Fix it by teaching objtool to detect when a non-relocated jump target is
a return thunk (or retpoline).

Fixes: e92626af3234 ("x86/retpoline: Remove .text..__x86.return_thunk section")
Reported-by: David Kaplan <david.kaplan@amd.com>
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Link: https://lore.kernel.org/r/20231012024737.eg5phclogp67ik6x@treble
---
 tools/objtool/check.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index e308d1b..e94756e 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1611,6 +1611,22 @@ static int add_jump_destinations(struct objtool_file *file)
 		}
 
 		/*
+		 * An intra-TU jump in retpoline.o might not have a relocation
+		 * for its jump dest, in which case the above
+		 * add_{retpoline,return}_call() didn't happen.
+		 */
+		if (jump_dest->sym && jump_dest->offset == jump_dest->sym->offset) {
+			if (jump_dest->sym->retpoline_thunk) {
+				add_retpoline_call(file, insn);
+				continue;
+			}
+			if (jump_dest->sym->return_thunk) {
+				add_return_call(file, insn, true);
+				continue;
+			}
+		}
+
+		/*
 		 * Cross-function jump.
 		 */
 		if (insn_func(insn) && insn_func(jump_dest) &&