* [PATCH v2 1/1] ARM: kprobes: Explicitly reserve r7 for local variables
@ 2023-11-15 9:58 Maria Yu
2023-11-16 11:11 ` Ard Biesheuvel
2023-11-16 17:24 ` Nathan Chancellor
0 siblings, 2 replies; 5+ messages in thread
From: Maria Yu @ 2023-11-15 9:58 UTC (permalink / raw)
To: linux, ardb, mhiramat
Cc: Maria Yu, kernel, linux-arm-kernel, linux-arm-msm, linux-kernel,
quic_lijuang, stable
Registers r7 is removed in clobber list, so compiler may choose r7 for
local variables usage, while r7 will be actually updated by the inline asm
code. This caused the runtime behavior wrong.
While those kind of reserved registers cannot be set to clobber list
because of error like "inline asm clobber list contains reserved
registers".
Explicitly reserve r7 by adding attribute no-omit-frame-pointer for needed
function, then in T32 asm code r7 is used as a frame pointer and is not
available for use as a general-purpose register.
Note that "no-omit-frame-pointer" will make the code size a little bigger
to store the stack frame pointer. So limited to needed functions can have
the less impact than the full source file.
Fixes: dd12e97f3c72 ("ARM: kprobes: treat R7 as the frame pointer register in Thumb2 builds")
Signed-off-by: Maria Yu <quic_aiquny@quicinc.com>
Cc: stable@vger.kernel.org
---
arch/arm/probes/kprobes/actions-thumb.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm/probes/kprobes/actions-thumb.c b/arch/arm/probes/kprobes/actions-thumb.c
index 51624fc263fc..c2fdaf9f6dba 100644
--- a/arch/arm/probes/kprobes/actions-thumb.c
+++ b/arch/arm/probes/kprobes/actions-thumb.c
@@ -438,7 +438,7 @@ t16_simulate_branch(probes_opcode_t insn,
regs->ARM_pc = pc + (offset * 2);
}
-static unsigned long __kprobes
+static unsigned long __kprobes __attribute__((optimize("no-omit-frame-pointer")))
t16_emulate_loregs(probes_opcode_t insn,
struct arch_probes_insn *asi, struct pt_regs *regs)
{
@@ -521,7 +521,7 @@ t16_decode_hiregs(probes_opcode_t insn, struct arch_probes_insn *asi,
return INSN_GOOD;
}
-static void __kprobes
+static void __kprobes __attribute__((optimize("no-omit-frame-pointer")))
t16_emulate_push(probes_opcode_t insn,
struct arch_probes_insn *asi, struct pt_regs *regs)
{
@@ -557,7 +557,7 @@ t16_decode_push(probes_opcode_t insn, struct arch_probes_insn *asi,
return INSN_GOOD;
}
-static void __kprobes
+static void __kprobes __attribute__((optimize("no-omit-frame-pointer")))
t16_emulate_pop_nopc(probes_opcode_t insn,
struct arch_probes_insn *asi, struct pt_regs *regs)
{
@@ -576,7 +576,7 @@ t16_emulate_pop_nopc(probes_opcode_t insn,
);
}
-static void __kprobes
+static void __kprobes __attribute__((optimize("no-omit-frame-pointer")))
t16_emulate_pop_pc(probes_opcode_t insn,
struct arch_probes_insn *asi, struct pt_regs *regs)
{
base-commit: 9bacdd8996c77c42ca004440be610692275ff9d0
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v2 1/1] ARM: kprobes: Explicitly reserve r7 for local variables
2023-11-15 9:58 [PATCH v2 1/1] ARM: kprobes: Explicitly reserve r7 for local variables Maria Yu
@ 2023-11-16 11:11 ` Ard Biesheuvel
2023-11-16 17:24 ` Nathan Chancellor
1 sibling, 0 replies; 5+ messages in thread
From: Ard Biesheuvel @ 2023-11-16 11:11 UTC (permalink / raw)
To: Maria Yu
Cc: linux, mhiramat, kernel, linux-arm-kernel, linux-arm-msm,
linux-kernel, quic_lijuang, stable
On Wed, 15 Nov 2023 at 19:58, Maria Yu <quic_aiquny@quicinc.com> wrote:
>
> Registers r7 is removed in clobber list, so compiler may choose r7 for
> local variables usage, while r7 will be actually updated by the inline asm
> code. This caused the runtime behavior wrong.
> While those kind of reserved registers cannot be set to clobber list
> because of error like "inline asm clobber list contains reserved
> registers".
> Explicitly reserve r7 by adding attribute no-omit-frame-pointer for needed
> function, then in T32 asm code r7 is used as a frame pointer and is not
> available for use as a general-purpose register.
> Note that "no-omit-frame-pointer" will make the code size a little bigger
> to store the stack frame pointer. So limited to needed functions can have
> the less impact than the full source file.
>
> Fixes: dd12e97f3c72 ("ARM: kprobes: treat R7 as the frame pointer register in Thumb2 builds")
> Signed-off-by: Maria Yu <quic_aiquny@quicinc.com>
> Cc: stable@vger.kernel.org
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
> ---
> arch/arm/probes/kprobes/actions-thumb.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/probes/kprobes/actions-thumb.c b/arch/arm/probes/kprobes/actions-thumb.c
> index 51624fc263fc..c2fdaf9f6dba 100644
> --- a/arch/arm/probes/kprobes/actions-thumb.c
> +++ b/arch/arm/probes/kprobes/actions-thumb.c
> @@ -438,7 +438,7 @@ t16_simulate_branch(probes_opcode_t insn,
> regs->ARM_pc = pc + (offset * 2);
> }
>
> -static unsigned long __kprobes
> +static unsigned long __kprobes __attribute__((optimize("no-omit-frame-pointer")))
> t16_emulate_loregs(probes_opcode_t insn,
> struct arch_probes_insn *asi, struct pt_regs *regs)
> {
> @@ -521,7 +521,7 @@ t16_decode_hiregs(probes_opcode_t insn, struct arch_probes_insn *asi,
> return INSN_GOOD;
> }
>
> -static void __kprobes
> +static void __kprobes __attribute__((optimize("no-omit-frame-pointer")))
> t16_emulate_push(probes_opcode_t insn,
> struct arch_probes_insn *asi, struct pt_regs *regs)
> {
> @@ -557,7 +557,7 @@ t16_decode_push(probes_opcode_t insn, struct arch_probes_insn *asi,
> return INSN_GOOD;
> }
>
> -static void __kprobes
> +static void __kprobes __attribute__((optimize("no-omit-frame-pointer")))
> t16_emulate_pop_nopc(probes_opcode_t insn,
> struct arch_probes_insn *asi, struct pt_regs *regs)
> {
> @@ -576,7 +576,7 @@ t16_emulate_pop_nopc(probes_opcode_t insn,
> );
> }
>
> -static void __kprobes
> +static void __kprobes __attribute__((optimize("no-omit-frame-pointer")))
> t16_emulate_pop_pc(probes_opcode_t insn,
> struct arch_probes_insn *asi, struct pt_regs *regs)
> {
>
> base-commit: 9bacdd8996c77c42ca004440be610692275ff9d0
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2 1/1] ARM: kprobes: Explicitly reserve r7 for local variables
2023-11-15 9:58 [PATCH v2 1/1] ARM: kprobes: Explicitly reserve r7 for local variables Maria Yu
2023-11-16 11:11 ` Ard Biesheuvel
@ 2023-11-16 17:24 ` Nathan Chancellor
2023-11-16 17:36 ` Ard Biesheuvel
1 sibling, 1 reply; 5+ messages in thread
From: Nathan Chancellor @ 2023-11-16 17:24 UTC (permalink / raw)
To: Maria Yu
Cc: linux, ardb, mhiramat, kernel, linux-arm-kernel, linux-arm-msm,
linux-kernel, quic_lijuang, stable
On Wed, Nov 15, 2023 at 05:58:30PM +0800, Maria Yu wrote:
> Registers r7 is removed in clobber list, so compiler may choose r7 for
> local variables usage, while r7 will be actually updated by the inline asm
> code. This caused the runtime behavior wrong.
> While those kind of reserved registers cannot be set to clobber list
> because of error like "inline asm clobber list contains reserved
> registers".
> Explicitly reserve r7 by adding attribute no-omit-frame-pointer for needed
> function, then in T32 asm code r7 is used as a frame pointer and is not
> available for use as a general-purpose register.
> Note that "no-omit-frame-pointer" will make the code size a little bigger
> to store the stack frame pointer. So limited to needed functions can have
> the less impact than the full source file.
>
> Fixes: dd12e97f3c72 ("ARM: kprobes: treat R7 as the frame pointer register in Thumb2 builds")
> Signed-off-by: Maria Yu <quic_aiquny@quicinc.com>
> Cc: stable@vger.kernel.org
This causes warnings with clang:
arch/arm/probes/kprobes/actions-thumb.c:441:47: warning: unknown attribute 'optimize' ignored [-Wunknown-attributes]
441 | static unsigned long __kprobes __attribute__((optimize("no-omit-frame-pointer")))
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
arch/arm/probes/kprobes/actions-thumb.c:524:38: warning: unknown attribute 'optimize' ignored [-Wunknown-attributes]
524 | static void __kprobes __attribute__((optimize("no-omit-frame-pointer")))
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
arch/arm/probes/kprobes/actions-thumb.c:560:38: warning: unknown attribute 'optimize' ignored [-Wunknown-attributes]
560 | static void __kprobes __attribute__((optimize("no-omit-frame-pointer")))
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
arch/arm/probes/kprobes/actions-thumb.c:579:38: warning: unknown attribute 'optimize' ignored [-Wunknown-attributes]
579 | static void __kprobes __attribute__((optimize("no-omit-frame-pointer")))
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
4 warnings generated.
Furthermore, as far as I am aware, the optimize attribute has other issues so
its use is discouraged, see commits 080b6f407635 ("bpf: Don't rely on GCC
__attribute__((optimize)) to disable GCSE") and a7223f5bfcae ("powerpc: Avoid
broken GCC __attribute__((optimize))").
Cheers,
Nathan
> ---
> arch/arm/probes/kprobes/actions-thumb.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/probes/kprobes/actions-thumb.c b/arch/arm/probes/kprobes/actions-thumb.c
> index 51624fc263fc..c2fdaf9f6dba 100644
> --- a/arch/arm/probes/kprobes/actions-thumb.c
> +++ b/arch/arm/probes/kprobes/actions-thumb.c
> @@ -438,7 +438,7 @@ t16_simulate_branch(probes_opcode_t insn,
> regs->ARM_pc = pc + (offset * 2);
> }
>
> -static unsigned long __kprobes
> +static unsigned long __kprobes __attribute__((optimize("no-omit-frame-pointer")))
> t16_emulate_loregs(probes_opcode_t insn,
> struct arch_probes_insn *asi, struct pt_regs *regs)
> {
> @@ -521,7 +521,7 @@ t16_decode_hiregs(probes_opcode_t insn, struct arch_probes_insn *asi,
> return INSN_GOOD;
> }
>
> -static void __kprobes
> +static void __kprobes __attribute__((optimize("no-omit-frame-pointer")))
> t16_emulate_push(probes_opcode_t insn,
> struct arch_probes_insn *asi, struct pt_regs *regs)
> {
> @@ -557,7 +557,7 @@ t16_decode_push(probes_opcode_t insn, struct arch_probes_insn *asi,
> return INSN_GOOD;
> }
>
> -static void __kprobes
> +static void __kprobes __attribute__((optimize("no-omit-frame-pointer")))
> t16_emulate_pop_nopc(probes_opcode_t insn,
> struct arch_probes_insn *asi, struct pt_regs *regs)
> {
> @@ -576,7 +576,7 @@ t16_emulate_pop_nopc(probes_opcode_t insn,
> );
> }
>
> -static void __kprobes
> +static void __kprobes __attribute__((optimize("no-omit-frame-pointer")))
> t16_emulate_pop_pc(probes_opcode_t insn,
> struct arch_probes_insn *asi, struct pt_regs *regs)
> {
>
> base-commit: 9bacdd8996c77c42ca004440be610692275ff9d0
> --
> 2.17.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2 1/1] ARM: kprobes: Explicitly reserve r7 for local variables
2023-11-16 17:24 ` Nathan Chancellor
@ 2023-11-16 17:36 ` Ard Biesheuvel
2023-11-20 2:00 ` Aiqun(Maria) Yu
0 siblings, 1 reply; 5+ messages in thread
From: Ard Biesheuvel @ 2023-11-16 17:36 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Maria Yu, linux, mhiramat, kernel, linux-arm-kernel,
linux-arm-msm, linux-kernel, quic_lijuang, stable
On Fri, 17 Nov 2023 at 03:24, Nathan Chancellor <nathan@kernel.org> wrote:
>
> On Wed, Nov 15, 2023 at 05:58:30PM +0800, Maria Yu wrote:
> > Registers r7 is removed in clobber list, so compiler may choose r7 for
> > local variables usage, while r7 will be actually updated by the inline asm
> > code. This caused the runtime behavior wrong.
> > While those kind of reserved registers cannot be set to clobber list
> > because of error like "inline asm clobber list contains reserved
> > registers".
> > Explicitly reserve r7 by adding attribute no-omit-frame-pointer for needed
> > function, then in T32 asm code r7 is used as a frame pointer and is not
> > available for use as a general-purpose register.
> > Note that "no-omit-frame-pointer" will make the code size a little bigger
> > to store the stack frame pointer. So limited to needed functions can have
> > the less impact than the full source file.
> >
> > Fixes: dd12e97f3c72 ("ARM: kprobes: treat R7 as the frame pointer register in Thumb2 builds")
> > Signed-off-by: Maria Yu <quic_aiquny@quicinc.com>
> > Cc: stable@vger.kernel.org
>
> This causes warnings with clang:
>
> arch/arm/probes/kprobes/actions-thumb.c:441:47: warning: unknown attribute 'optimize' ignored [-Wunknown-attributes]
> 441 | static unsigned long __kprobes __attribute__((optimize("no-omit-frame-pointer")))
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> arch/arm/probes/kprobes/actions-thumb.c:524:38: warning: unknown attribute 'optimize' ignored [-Wunknown-attributes]
> 524 | static void __kprobes __attribute__((optimize("no-omit-frame-pointer")))
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> arch/arm/probes/kprobes/actions-thumb.c:560:38: warning: unknown attribute 'optimize' ignored [-Wunknown-attributes]
> 560 | static void __kprobes __attribute__((optimize("no-omit-frame-pointer")))
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> arch/arm/probes/kprobes/actions-thumb.c:579:38: warning: unknown attribute 'optimize' ignored [-Wunknown-attributes]
> 579 | static void __kprobes __attribute__((optimize("no-omit-frame-pointer")))
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 4 warnings generated.
>
> Furthermore, as far as I am aware, the optimize attribute has other issues so
> its use is discouraged, see commits 080b6f407635 ("bpf: Don't rely on GCC
> __attribute__((optimize)) to disable GCSE") and a7223f5bfcae ("powerpc: Avoid
> broken GCC __attribute__((optimize))").
>
Ah yes, apalogies for missing that. I did a test build with Clang but
I did not spot the errors. So
Unreviewed-by: ....
Maria, please use the Makefile based per-file CFLAGS override that I
suggested before. There is really no reason to make this per-function.
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2 1/1] ARM: kprobes: Explicitly reserve r7 for local variables
2023-11-16 17:36 ` Ard Biesheuvel
@ 2023-11-20 2:00 ` Aiqun(Maria) Yu
0 siblings, 0 replies; 5+ messages in thread
From: Aiqun(Maria) Yu @ 2023-11-20 2:00 UTC (permalink / raw)
To: Ard Biesheuvel, Nathan Chancellor
Cc: linux, mhiramat, kernel, linux-arm-kernel, linux-arm-msm,
linux-kernel, quic_lijuang, stable
On 11/17/2023 1:36 AM, Ard Biesheuvel wrote:
> On Fri, 17 Nov 2023 at 03:24, Nathan Chancellor <nathan@kernel.org> wrote:
>>
>> On Wed, Nov 15, 2023 at 05:58:30PM +0800, Maria Yu wrote:
>>> Registers r7 is removed in clobber list, so compiler may choose r7 for
>>> local variables usage, while r7 will be actually updated by the inline asm
>>> code. This caused the runtime behavior wrong.
>>> While those kind of reserved registers cannot be set to clobber list
>>> because of error like "inline asm clobber list contains reserved
>>> registers".
>>> Explicitly reserve r7 by adding attribute no-omit-frame-pointer for needed
>>> function, then in T32 asm code r7 is used as a frame pointer and is not
>>> available for use as a general-purpose register.
>>> Note that "no-omit-frame-pointer" will make the code size a little bigger
>>> to store the stack frame pointer. So limited to needed functions can have
>>> the less impact than the full source file.
>>>
>>> Fixes: dd12e97f3c72 ("ARM: kprobes: treat R7 as the frame pointer register in Thumb2 builds")
>>> Signed-off-by: Maria Yu <quic_aiquny@quicinc.com>
>>> Cc: stable@vger.kernel.org
>>
>> This causes warnings with clang:
>>
>> arch/arm/probes/kprobes/actions-thumb.c:441:47: warning: unknown attribute 'optimize' ignored [-Wunknown-attributes]
>> 441 | static unsigned long __kprobes __attribute__((optimize("no-omit-frame-pointer")))
>> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> arch/arm/probes/kprobes/actions-thumb.c:524:38: warning: unknown attribute 'optimize' ignored [-Wunknown-attributes]
>> 524 | static void __kprobes __attribute__((optimize("no-omit-frame-pointer")))
>> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> arch/arm/probes/kprobes/actions-thumb.c:560:38: warning: unknown attribute 'optimize' ignored [-Wunknown-attributes]
>> 560 | static void __kprobes __attribute__((optimize("no-omit-frame-pointer")))
>> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> arch/arm/probes/kprobes/actions-thumb.c:579:38: warning: unknown attribute 'optimize' ignored [-Wunknown-attributes]
>> 579 | static void __kprobes __attribute__((optimize("no-omit-frame-pointer")))
>> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> 4 warnings generated.
>>
>> Furthermore, as far as I am aware, the optimize attribute has other issues so
>> its use is discouraged, see commits 080b6f407635 ("bpf: Don't rely on GCC
>> __attribute__((optimize)) to disable GCSE") and a7223f5bfcae ("powerpc: Avoid
>> broken GCC __attribute__((optimize))").
Thx for trying this. I was tested with arm-linux-gnueabihf-gcc and not
notice this.
>>
>
> Ah yes, apalogies for missing that. I did a test build with Clang but
> I did not spot the errors. So
>
> Unreviewed-by: ....
>
> Maria, please use the Makefile based per-file CFLAGS override that I
> suggested before. There is really no reason to make this per-function.
Ok. I will upload a new patchset.
--
Thx and BRs,
Aiqun(Maria) Yu
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-11-20 2:01 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-15 9:58 [PATCH v2 1/1] ARM: kprobes: Explicitly reserve r7 for local variables Maria Yu
2023-11-16 11:11 ` Ard Biesheuvel
2023-11-16 17:24 ` Nathan Chancellor
2023-11-16 17:36 ` Ard Biesheuvel
2023-11-20 2:00 ` Aiqun(Maria) Yu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox