| Message ID | 20211126032249.1652080-1-npiggin@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/3] powerpc/code-patching: work around code patching verification in patching tests | expand |
Le 26/11/2021 à 04:22, Nicholas Piggin a écrit : > Code patching tests patch the stack and (non-module) vmalloc space now, > which falls afoul of the new address check. > > The stack patching can easily be fixed, but the vmalloc patching is more > difficult. For now, add an ugly workaround to skip the check while the > test code is running. This really looks hacky. To skip the test, you can call do_patch_instruction() instead of calling patch_instruction(). > > Fixes: 8b8a8f0ab3f55 ("powerpc/code-patching: Improve verification of patchability") > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > --- > arch/powerpc/lib/code-patching.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c > index 5e2fe133639e..57e160963ab7 100644 > --- a/arch/powerpc/lib/code-patching.c > +++ b/arch/powerpc/lib/code-patching.c > @@ -187,10 +187,12 @@ static int do_patch_instruction(u32 *addr, struct ppc_inst instr) > > #endif /* CONFIG_STRICT_KERNEL_RWX */ > > +static bool skip_addr_verif = false; > + > int patch_instruction(u32 *addr, struct ppc_inst instr) > { > /* Make sure we aren't patching a freed init section */ > - if (!kernel_text_address((unsigned long)addr)) > + if (!skip_addr_verif && !kernel_text_address((unsigned long)addr)) > return 0; > > return do_patch_instruction(addr, instr); > @@ -738,11 +740,13 @@ static int __init test_code_patching(void) > { > printk(KERN_DEBUG "Running code patching self-tests ...\n"); > > + skip_addr_verif = true; > test_branch_iform(); > test_branch_bform(); > test_create_function_call(); > test_translate_branch(); > test_prefixed_patching(); > + skip_addr_verif = false; > > return 0; > } >
Excerpts from Christophe Leroy's message of November 26, 2021 4:34 pm: > > > Le 26/11/2021 à 04:22, Nicholas Piggin a écrit : >> Code patching tests patch the stack and (non-module) vmalloc space now, >> which falls afoul of the new address check. >> >> The stack patching can easily be fixed, but the vmalloc patching is more >> difficult. For now, add an ugly workaround to skip the check while the >> test code is running. > > This really looks hacky. > > To skip the test, you can call do_patch_instruction() instead of calling > patch_instruction(). And make a do_patch_branch function. I thought about it, and thought this is sligtly easier. Thanks, Nick
Le 26/11/2021 à 11:27, Nicholas Piggin a écrit : > Excerpts from Christophe Leroy's message of November 26, 2021 4:34 pm: >> >> >> Le 26/11/2021 à 04:22, Nicholas Piggin a écrit : >>> Code patching tests patch the stack and (non-module) vmalloc space now, >>> which falls afoul of the new address check. >>> >>> The stack patching can easily be fixed, but the vmalloc patching is more >>> difficult. For now, add an ugly workaround to skip the check while the >>> test code is running. >> >> This really looks hacky. >> >> To skip the test, you can call do_patch_instruction() instead of calling >> patch_instruction(). > > And make a do_patch_branch function. I thought about it, and thought > this is sligtly easier. > Anyway, as reported by Sachin the ftrace code also trips in the new verification. So I have submitted a patch to revert to the previous level of verification. Then we can fix all this properly without going through a temporary hack and activate the verification again once every caller is fixed. I was not able to reproduce Sachin's problem on PPC32. Could it be specific to PPC64 ? Christophe
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c index 5e2fe133639e..57e160963ab7 100644 --- a/arch/powerpc/lib/code-patching.c +++ b/arch/powerpc/lib/code-patching.c @@ -187,10 +187,12 @@ static int do_patch_instruction(u32 *addr, struct ppc_inst instr) #endif /* CONFIG_STRICT_KERNEL_RWX */ +static bool skip_addr_verif = false; + int patch_instruction(u32 *addr, struct ppc_inst instr) { /* Make sure we aren't patching a freed init section */ - if (!kernel_text_address((unsigned long)addr)) + if (!skip_addr_verif && !kernel_text_address((unsigned long)addr)) return 0; return do_patch_instruction(addr, instr); @@ -738,11 +740,13 @@ static int __init test_code_patching(void) { printk(KERN_DEBUG "Running code patching self-tests ...\n"); + skip_addr_verif = true; test_branch_iform(); test_branch_bform(); test_create_function_call(); test_translate_branch(); test_prefixed_patching(); + skip_addr_verif = false; return 0; }
Code patching tests patch the stack and (non-module) vmalloc space now, which falls afoul of the new address check. The stack patching can easily be fixed, but the vmalloc patching is more difficult. For now, add an ugly workaround to skip the check while the test code is running. Fixes: 8b8a8f0ab3f55 ("powerpc/code-patching: Improve verification of patchability") Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- arch/powerpc/lib/code-patching.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)