Messages in this thread | | | Date | Tue, 17 Jan 2023 09:14:38 -0800 | Subject | Re: [PATCH] tools: bpf: Disable stack protector | From | Yonghong Song <> |
| |
On 1/17/23 8:31 AM, Jose E. Marchesi wrote: > >>> On 1/16/23 2:49 PM, Peter Foley wrote: >>>> On Mon, Jan 16, 2023 at 4:59 AM Eduard Zingerman <eddyz87@gmail.com> wrote: >>>>> >>>>> A bit tangential, but since BPF LLVM backend does not support the >>>>> stack protector (should it?) there is also an option to adjust LLVM >>>>> to avoid this instrumentation, WDYT? >>>>> >>>> That would probably be worth doing, yes. >>>> But given that won't help already released versions of clang, it >>>> should probably happen in addition to this patch. >>> >>> Peter, >>> >>> If I understand correctly (by inspecting clang code), the stack >>> protector is off by default. Do you have link to Gentoo build >>> page to show how they enable stack protector? cmake config or >>> a private patch? >>> >>> Jose, >>> >>> How gcc-bpf handle stack protector? The compiler just disables >>> stack protector for bpf target? >> >> It doesn't. -fstack-protector is disabled by default in GCC. When you >> use it you get something like: >> >> $ echo 'int foo() { char s[256]; return s[3]; }' | bpf-unknown-none-gcc \ >> -fstack-protector -S -o foo.s -O2 -xc - >> $ cat foo.s >> .file "<stdin>" >> .text >> .align 3 >> .global foo >> .type foo, @function >> foo: >> lddw %r1,__stack_chk_guard >> ldxdw %r0,[%r1+0] >> stxdw [%fp+-8],%r0 >> ldxb %r0,[%fp+-261] >> lsh %r0,56 >> arsh %r0,56 >> ldxdw %r2,[%fp+-8] >> ldxdw %r3,[%r1+0] >> jne %r2,%r3,.L4 >> exit >> .L4: >> call __stack_chk_fail >> .size foo, .-foo >> .ident "GCC: (GNU) 12.0.0 20211206 (experimental)" >> >> i.e. it pushes a stack canary and checks it upon function exit, calling >> __stack_chk_fail. >> >> If clang has -fstack-protector ON by default and you change the BPF >> backend in order to ignore the flag, I think we should do the same in >> GCC. > > I went ahead and pushed the patch below to GCC master. If > -fstack-protector is ever considered useful in the architecture, we can > always stop disabling it. > > I would recommend to change the default for -fstack-protector in clang > to be off by default when targetting BPF targets, and to emit the same > or similar note to the user when the option is enabled explicitly with > -fstack-protector: > > note: ‘-fstack-protector’ does not work on this architecture > > WDYT? > > From 3b81f5c4d8e0d79cbd6927d004185707c14e54b2 Mon Sep 17 00:00:00 2001 > Date: Tue, 17 Jan 2023 17:16:32 +0100 > Subject: [COMMITTED] bpf: disable -fstack-protector in BPF > > The stack protector is not supported in BPF. This patch disables > -fstack-protector in bpf-* targets, along with the emission of a note > indicating that the feature is not supported in this platform. > > Regtested in bpf-unknown-none. > > gcc/ChangeLog: > > * config/bpf/bpf.cc (bpf_option_override): Disable > -fstack-protector. > --- > gcc/config/bpf/bpf.cc | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/gcc/config/bpf/bpf.cc b/gcc/config/bpf/bpf.cc > index 576a1fe8eab..b268801d00c 100644 > --- a/gcc/config/bpf/bpf.cc > +++ b/gcc/config/bpf/bpf.cc > @@ -253,6 +253,14 @@ bpf_option_override (void) > if (bpf_has_jmp32 == -1) > bpf_has_jmp32 = (bpf_isa >= ISA_V3); > > + /* Disable -fstack-protector as it is not supported in BPF. */ > + if (flag_stack_protect) > + { > + inform (input_location, > + "%<-fstack-protector%> does not work " > + " on this architecture"); > + flag_stack_protect = 0; > + } > }
Thanks, just replied based on a previous email communication a while back. Yes, clang could do similar things.
> > #undef TARGET_OPTION_OVERRIDE
| |