Skip to contentSkip to conversation

Patch "powerpc/modules: Don't try to restore r2 after a sibling call" has been added to the 4.14-stable tree

PATCH [email protected] 1 participant 1 message Mar 16, 2018

Subsystems (2)
lore
Share & tools
lore

[email protected]

author maintainer Applied
#0 lore raw

This is a note to let you know that I've just added the patch titled

powerpc/modules: Don't try to restore r2 after a sibling call

to the 4.14-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
powerpc-modules-don-t-try-to-restore-r2-after-a-sibling-call.patch
and it can be found in the queue-4.14 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

Quoted 1 line

From foo@baz Fri Mar 16 15:43:17 CET 2018

From: Josh Poimboeuf <[email protected]>
Date: Thu, 16 Nov 2017 11:45:37 -0600
Subject: powerpc/modules: Don't try to restore r2 after a sibling call

From: Josh Poimboeuf <[email protected]>

[ Upstream commit b9eab08d012fa093947b230f9a87257c27fb829b ]

When attempting to load a livepatch module, I got the following error:

module_64: patch_module: Expect noop after relocate, got 3c820000

The error was triggered by the following code in
unregister_netdevice_queue():

14c: 00 00 00 48 b 14c <unregister_netdevice_queue+0x14c>

                         14c: R_PPC64_REL24      net_set_todo

150: 00 00 82 3c addis r4,r2,0

GCC didn't insert a nop after the branch to net_set_todo() because it's a sibling call, so it never returns. The nop isn't needed after the branch in that case.

---
 arch/powerpc/include/asm/code-patching.h |    1 +
 arch/powerpc/kernel/module_64.c          |   12 +++++++++++-
 arch/powerpc/lib/code-patching.c         |    5 +++++
 3 files changed, 17 insertions(+), 1 deletion(-)
arch/powerpc/include/asm/code-patching.h +1 -0
@@ -33,6 +33,7 @@ int patch_branch(unsigned int *addr, uns
 int patch_instruction(unsigned int *addr, unsigned int instr);
 
 int instr_is_relative_branch(unsigned int instr);
+int instr_is_relative_link_branch(unsigned int instr);
 int instr_is_branch_to_addr(const unsigned int *instr, unsigned long addr);
 unsigned long branch_target(const unsigned int *instr);
 unsigned int translate_branch(const unsigned int *dest,
arch/powerpc/kernel/module_64.c +11 -1
@@ -486,7 +486,17 @@ static bool is_early_mcount_callsite(u32
    restore r2. */
 static int restore_r2(u32 *instruction, struct module *me)
 {
-	if (is_early_mcount_callsite(instruction - 1))
+	u32 *prev_insn = instruction - 1;
+
+	if (is_early_mcount_callsite(prev_insn))
+		return 1;
+
+	/*
+	 * Make sure the branch isn't a sibling call.  Sibling calls aren't
+	 * "link" branches and they don't return, so they don't need the r2
+	 * restore afterwards.
+	 */
+	if (!instr_is_relative_link_branch(*prev_insn))
 		return 1;
 
 	if (*instruction != PPC_INST_NOP) {
arch/powerpc/lib/code-patching.c +5 -0
@@ -302,6 +302,11 @@ int instr_is_relative_branch(unsigned in
 	return instr_is_branch_iform(instr) || instr_is_branch_bform(instr);
 }
 
+int instr_is_relative_link_branch(unsigned int instr)
+{
+	return instr_is_relative_branch(instr) && (instr & BRANCH_SET_LINK);
+}
+
 static unsigned long branch_iform_target(const unsigned int *instr)
 {
 	signed long imm;

Patches currently in stable-queue which might be from [email protected] are

queue-4.14/powerpc-modules-don-t-try-to-restore-r2-after-a-sibling-call.patch