* [PATCH 1/3] powerpc/code-patching: work around code patching verification in patching tests
@ 2021-11-26 3:22 Nicholas Piggin
2021-11-26 3:22 ` [PATCH 2/3] powerpc/code-patching: warn on code patching failure Nicholas Piggin
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Nicholas Piggin @ 2021-11-26 3:22 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin
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(-)
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;
}
--
2.23.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] powerpc/code-patching: warn on code patching failure
2021-11-26 3:22 [PATCH 1/3] powerpc/code-patching: work around code patching verification in patching tests Nicholas Piggin
@ 2021-11-26 3:22 ` Nicholas Piggin
2021-11-26 3:22 ` [PATCH 3/3] powerpc/code-patching: don't use the stack for code patching tests Nicholas Piggin
2021-11-26 6:34 ` [PATCH 1/3] powerpc/code-patching: work around code patching verification in " Christophe Leroy
2 siblings, 0 replies; 6+ messages in thread
From: Nicholas Piggin @ 2021-11-26 3:22 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin
Callers are supposed to handle this, but it is possible that they
don't or they do but don't make much noise about it. A failure is
probably an indication of a bigger problem somewhere so it is good
to warn once about it.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/lib/code-patching.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 57e160963ab7..70247fc58b6e 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -161,6 +161,7 @@ static int do_patch_instruction(u32 *addr, struct ppc_inst instr)
text_poke_addr = (unsigned long)__this_cpu_read(text_poke_area)->addr;
if (map_patch_area(addr, text_poke_addr)) {
+ WARN_ON_ONCE(1);
err = -1;
goto out;
}
@@ -192,8 +193,10 @@ 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 (!skip_addr_verif && !kernel_text_address((unsigned long)addr))
+ if (!skip_addr_verif && !kernel_text_address((unsigned long)addr)) {
+ WARN_ON_ONCE(1);
return 0;
+ }
return do_patch_instruction(addr, instr);
}
--
2.23.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] powerpc/code-patching: don't use the stack for code patching tests
2021-11-26 3:22 [PATCH 1/3] powerpc/code-patching: work around code patching verification in patching tests Nicholas Piggin
2021-11-26 3:22 ` [PATCH 2/3] powerpc/code-patching: warn on code patching failure Nicholas Piggin
@ 2021-11-26 3:22 ` Nicholas Piggin
2021-11-26 6:34 ` [PATCH 1/3] powerpc/code-patching: work around code patching verification in " Christophe Leroy
2 siblings, 0 replies; 6+ messages in thread
From: Nicholas Piggin @ 2021-11-26 3:22 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin
Use the existing test function for code patching tests instead of
writing to the stack. This means the address verification does not have
to be bypassed for these tests.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/lib/code-patching.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 70247fc58b6e..babf6b22adef 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -422,9 +422,11 @@ static void __init test_branch_iform(void)
{
int err;
struct ppc_inst instr;
- u32 tmp[2];
- u32 *iptr = tmp;
- unsigned long addr = (unsigned long)tmp;
+ u32 *iptr;
+ unsigned long addr;
+
+ iptr = (u32 *)ppc_function_entry(test_trampoline);
+ addr = (unsigned long)iptr;
/* The simplest case, branch to self, no flags */
check(instr_is_branch_iform(ppc_inst(0x48000000)));
@@ -516,12 +518,12 @@ static void __init test_create_function_call(void)
static void __init test_branch_bform(void)
{
int err;
- unsigned long addr;
struct ppc_inst instr;
- u32 tmp[2];
- u32 *iptr = tmp;
+ u32 *iptr;
+ unsigned long addr;
unsigned int flags;
+ iptr = (u32 *)ppc_function_entry(test_trampoline);
addr = (unsigned long)iptr;
/* The simplest case, branch to self, no flags */
@@ -603,6 +605,12 @@ static void __init test_translate_branch(void)
if (!buf)
return;
+ /*
+ * Have to disable the address bounds check for patch_instruction
+ * because we are patching vmalloc space here.
+ */
+ skip_addr_verif = true;
+
/* Simple case, branch to self moved a little */
p = buf;
addr = (unsigned long)p;
@@ -715,6 +723,8 @@ static void __init test_translate_branch(void)
check(instr_is_branch_to_addr(p, addr));
check(instr_is_branch_to_addr(q, addr));
+ skip_addr_verif = false;
+
/* Free the buffer we were using */
vfree(buf);
}
@@ -743,13 +753,11 @@ 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;
}
--
2.23.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] powerpc/code-patching: work around code patching verification in patching tests
2021-11-26 3:22 [PATCH 1/3] powerpc/code-patching: work around code patching verification in patching tests Nicholas Piggin
2021-11-26 3:22 ` [PATCH 2/3] powerpc/code-patching: warn on code patching failure Nicholas Piggin
2021-11-26 3:22 ` [PATCH 3/3] powerpc/code-patching: don't use the stack for code patching tests Nicholas Piggin
@ 2021-11-26 6:34 ` Christophe Leroy
2021-11-26 10:27 ` Nicholas Piggin
2 siblings, 1 reply; 6+ messages in thread
From: Christophe Leroy @ 2021-11-26 6:34 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
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;
> }
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] powerpc/code-patching: work around code patching verification in patching tests
2021-11-26 6:34 ` [PATCH 1/3] powerpc/code-patching: work around code patching verification in " Christophe Leroy
@ 2021-11-26 10:27 ` Nicholas Piggin
2021-11-26 10:39 ` Christophe Leroy
0 siblings, 1 reply; 6+ messages in thread
From: Nicholas Piggin @ 2021-11-26 10:27 UTC (permalink / raw)
To: Christophe Leroy, linuxppc-dev
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] powerpc/code-patching: work around code patching verification in patching tests
2021-11-26 10:27 ` Nicholas Piggin
@ 2021-11-26 10:39 ` Christophe Leroy
0 siblings, 0 replies; 6+ messages in thread
From: Christophe Leroy @ 2021-11-26 10:39 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: Sachin Sant
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-11-26 10:39 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-26 3:22 [PATCH 1/3] powerpc/code-patching: work around code patching verification in patching tests Nicholas Piggin
2021-11-26 3:22 ` [PATCH 2/3] powerpc/code-patching: warn on code patching failure Nicholas Piggin
2021-11-26 3:22 ` [PATCH 3/3] powerpc/code-patching: don't use the stack for code patching tests Nicholas Piggin
2021-11-26 6:34 ` [PATCH 1/3] powerpc/code-patching: work around code patching verification in " Christophe Leroy
2021-11-26 10:27 ` Nicholas Piggin
2021-11-26 10:39 ` Christophe Leroy
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.