Le 05/09/2025 à 08:11, Andrew Donnellan a écrit :
If patch_branch() or patch_instruction() fails while updating a jump
label, we presently fail silently, leading to unpredictable behaviour
later on.
Change arch_jump_label_transform() to panic on a code patching failure,
matching the existing behaviour of arch_static_call_transform().
Reported-by: Erhard Furtner <erhar...@mailbox.org>
checkpatch.pl is not happy:
WARNING: Use lore.kernel.org archive links when possible - see
https://lore.kernel.org/lists.html
#131:
<https://lists.ozlabs.org/pipermail/linuxppc-dev/>
WARNING: Reported-by: should be immediately followed by Closes: with a
URL to the report
#173:
Reported-by: Erhard Furtner <erhar...@mailbox.org>
Signed-off-by: Andrew Donnellan <a...@linux.ibm.com>
total: 0 errors, 2 warnings, 0 checks, 16 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or
--fix-inplace.
/home/chleroy/Téléchargements/RFC-powerpc-Panic-on-jump-label-code-patching-failure.patch
has style problems, please review.
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
Signed-off-by: Andrew Donnellan <a...@linux.ibm.com>
Reviewed-by: Christophe Leroy <christophe.le...@csgroup.eu>
---
Ran into this while debugging an issue that Erhard reported to me about my
PAGE_TABLE_CHECK series on a G4, where updating a static key failed
silently, but only for one call site, leading to an incorrect reference
count later on. This looks to be due to the issue fixed in [0]. A loud
failure would have saved us all considerable debugging time.
Should I change the return type of arch_jump_label_transform() and handle
this in an arch-independent way? Are there other users of code patching
in powerpc that ought to be hardened?whon
I think all callers of patch_branch() and patch_instruction() should
check returned value. Several already do. I think we should fix the ones
which don't then make patch_branch() and patch_instruction() __must_check
Or is this excessive?
[0]
https://patchwork.ozlabs.org/project/linuxppc-dev/patch/4b5e6eb281d7b1ea77619bee17095f905a125168.1757003584.git.christophe.le...@csgroup.eu/
---
arch/powerpc/kernel/jump_label.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/jump_label.c b/arch/powerpc/kernel/jump_label.c
index 2659e1ac8604..80d41ed7ac50 100644
--- a/arch/powerpc/kernel/jump_label.c
+++ b/arch/powerpc/kernel/jump_label.c
@@ -12,9 +12,14 @@ void arch_jump_label_transform(struct jump_entry *entry,
enum jump_label_type type)
{
u32 *addr = (u32 *)jump_entry_code(entry);
+ int err;
if (type == JUMP_LABEL_JMP)
- patch_branch(addr, jump_entry_target(entry), 0);
+ err = patch_branch(addr, jump_entry_target(entry), 0);
else
- patch_instruction(addr, ppc_inst(PPC_RAW_NOP()));
+ err = patch_instruction(addr, ppc_inst(PPC_RAW_NOP()));
+
+ if (err)
+ panic("%s: patching failed, err %d, type %d, addr %pS, target
%pS\n",
+ __func__, err, type, addr, (void
*)jump_entry_target(entry));
}
