Avoid build errors on distros that force the stack protector on by
default.
e.g.
CLANG /home/peter/linux/work/tools/bpf/bpftool/pid_iter.bpf.o
skeleton/pid_iter.bpf.c:53:5: error: A call to built-in function '__stack_chk_fail' is not supported.
int iter(struct bpf_iter__task_file *ctx)
^
1 error generated.
Signed-off-by: Peter Foley <[email protected]>
---
tools/bpf/bpftool/Makefile | 1 +
tools/bpf/runqslower/Makefile | 5 +++--
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index f610e184ce02a..36ac0002e386f 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -215,6 +215,7 @@ $(OUTPUT)%.bpf.o: skeleton/%.bpf.c $(OUTPUT)vmlinux.h $(LIBBPF_BOOTSTRAP)
-I$(or $(OUTPUT),.) \
-I$(srctree)/tools/include/uapi/ \
-I$(LIBBPF_BOOTSTRAP_INCLUDE) \
+ -fno-stack-protector \
-g -O2 -Wall -target bpf -c $< -o $@
$(Q)$(LLVM_STRIP) -g $@
diff --git a/tools/bpf/runqslower/Makefile b/tools/bpf/runqslower/Makefile
index 8b3d87b82b7a2..f7313cc966a04 100644
--- a/tools/bpf/runqslower/Makefile
+++ b/tools/bpf/runqslower/Makefile
@@ -60,8 +60,9 @@ $(OUTPUT)/%.skel.h: $(OUTPUT)/%.bpf.o | $(BPFTOOL)
$(QUIET_GEN)$(BPFTOOL) gen skeleton $< > $@
$(OUTPUT)/%.bpf.o: %.bpf.c $(BPFOBJ) | $(OUTPUT)
- $(QUIET_GEN)$(CLANG) -g -O2 -target bpf $(INCLUDES) \
- -c $(filter %.c,$^) -o $@ && \
+ $(QUIET_GEN)$(CLANG) -g -O2 -target bpf $(INCLUDES) \
+ -fno-stack-protector \
+ -c $(filter %.c,$^) -o $@ && \
$(LLVM_STRIP) -g $@
$(OUTPUT)/%.o: %.c | $(OUTPUT)
---
base-commit: 97ec4d559d939743e8af83628be5af8da610d9dc
change-id: 20230114-bpf-918ae127b77a
Best regards,
--
Peter Foley <[email protected]>
2023-01-14 18:00 UTC-0500 ~ Peter Foley <[email protected]>
> Avoid build errors on distros that force the stack protector on by
> default.
> e.g.
> CLANG /home/peter/linux/work/tools/bpf/bpftool/pid_iter.bpf.o
> skeleton/pid_iter.bpf.c:53:5: error: A call to built-in function '__stack_chk_fail' is not supported.
> int iter(struct bpf_iter__task_file *ctx)
> ^
> 1 error generated.
>
> Signed-off-by: Peter Foley <[email protected]>
> ---
> tools/bpf/bpftool/Makefile | 1 +
> tools/bpf/runqslower/Makefile | 5 +++--
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> index f610e184ce02a..36ac0002e386f 100644
> --- a/tools/bpf/bpftool/Makefile
> +++ b/tools/bpf/bpftool/Makefile
> @@ -215,6 +215,7 @@ $(OUTPUT)%.bpf.o: skeleton/%.bpf.c $(OUTPUT)vmlinux.h $(LIBBPF_BOOTSTRAP)
> -I$(or $(OUTPUT),.) \
> -I$(srctree)/tools/include/uapi/ \
> -I$(LIBBPF_BOOTSTRAP_INCLUDE) \
> + -fno-stack-protector \
> -g -O2 -Wall -target bpf -c $< -o $@
> $(Q)$(LLVM_STRIP) -g $@
>
For bpftool, a similar patch was already submitted and merged to the
bpf-next tree last Friday: 878625e1c7a1 ("bpftool: Always disable stack
protection for BPF objects").
> diff --git a/tools/bpf/runqslower/Makefile b/tools/bpf/runqslower/Makefile
> index 8b3d87b82b7a2..f7313cc966a04 100644
> --- a/tools/bpf/runqslower/Makefile
> +++ b/tools/bpf/runqslower/Makefile
> @@ -60,8 +60,9 @@ $(OUTPUT)/%.skel.h: $(OUTPUT)/%.bpf.o | $(BPFTOOL)
> $(QUIET_GEN)$(BPFTOOL) gen skeleton $< > $@
>
> $(OUTPUT)/%.bpf.o: %.bpf.c $(BPFOBJ) | $(OUTPUT)
> - $(QUIET_GEN)$(CLANG) -g -O2 -target bpf $(INCLUDES) \
> - -c $(filter %.c,$^) -o $@ && \
> + $(QUIET_GEN)$(CLANG) -g -O2 -target bpf $(INCLUDES) \
> + -fno-stack-protector \
> + -c $(filter %.c,$^) -o $@ && \
> $(LLVM_STRIP) -g $@
>
> $(OUTPUT)/%.o: %.c | $(OUTPUT)
This one looks good, thanks!
I note a few more places in the repository where we compile to BPF using
clang. Given that there have been patches to add -fno-stack-protector at
several locations already, have you checked if any of these also need
the flag, by any chance, so we could fix this once and for all?
$ git grep -l 'target bpf ' | egrep -v '(Documentation|bpftool)'
kernel/bpf/preload/iterators/Makefile
samples/bpf/Makefile
samples/bpf/test_lwt_bpf.sh
tools/bpf/runqslower/Makefile
tools/build/feature/Makefile
tools/perf/Makefile.perf
tools/perf/util/llvm-utils.c
tools/testing/selftests/bpf/Makefile
tools/testing/selftests/net/bpf/Makefile
tools/testing/selftests/tc-testing/Makefile
On Mon, 2023-01-16 at 10:30 +0000, Quentin Monnet wrote:
> 2023-01-14 18:00 UTC-0500 ~ Peter Foley <[email protected]>
> > Avoid build errors on distros that force the stack protector on by
> > default.
> > e.g.
> > CLANG /home/peter/linux/work/tools/bpf/bpftool/pid_iter.bpf.o
> > skeleton/pid_iter.bpf.c:53:5: error: A call to built-in function '__stack_chk_fail' is not supported.
> > int iter(struct bpf_iter__task_file *ctx)
> > ^
> > 1 error generated.
> >
> > Signed-off-by: Peter Foley <[email protected]>
> > ---
> > tools/bpf/bpftool/Makefile | 1 +
> > tools/bpf/runqslower/Makefile | 5 +++--
> > 2 files changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> > index f610e184ce02a..36ac0002e386f 100644
> > --- a/tools/bpf/bpftool/Makefile
> > +++ b/tools/bpf/bpftool/Makefile
> > @@ -215,6 +215,7 @@ $(OUTPUT)%.bpf.o: skeleton/%.bpf.c $(OUTPUT)vmlinux.h $(LIBBPF_BOOTSTRAP)
> > -I$(or $(OUTPUT),.) \
> > -I$(srctree)/tools/include/uapi/ \
> > -I$(LIBBPF_BOOTSTRAP_INCLUDE) \
> > + -fno-stack-protector \
> > -g -O2 -Wall -target bpf -c $< -o $@
> > $(Q)$(LLVM_STRIP) -g $@
> >
>
> For bpftool, a similar patch was already submitted and merged to the
> bpf-next tree last Friday: 878625e1c7a1 ("bpftool: Always disable stack
> protection for BPF objects").
>
> > diff --git a/tools/bpf/runqslower/Makefile b/tools/bpf/runqslower/Makefile
> > index 8b3d87b82b7a2..f7313cc966a04 100644
> > --- a/tools/bpf/runqslower/Makefile
> > +++ b/tools/bpf/runqslower/Makefile
> > @@ -60,8 +60,9 @@ $(OUTPUT)/%.skel.h: $(OUTPUT)/%.bpf.o | $(BPFTOOL)
> > $(QUIET_GEN)$(BPFTOOL) gen skeleton $< > $@
> >
> > $(OUTPUT)/%.bpf.o: %.bpf.c $(BPFOBJ) | $(OUTPUT)
> > - $(QUIET_GEN)$(CLANG) -g -O2 -target bpf $(INCLUDES) \
> > - -c $(filter %.c,$^) -o $@ && \
> > + $(QUIET_GEN)$(CLANG) -g -O2 -target bpf $(INCLUDES) \
> > + -fno-stack-protector \
> > + -c $(filter %.c,$^) -o $@ && \
> > $(LLVM_STRIP) -g $@
> >
> > $(OUTPUT)/%.o: %.c | $(OUTPUT)
>
> This one looks good, thanks!
>
> I note a few more places in the repository where we compile to BPF using
> clang. Given that there have been patches to add -fno-stack-protector at
> several locations already, have you checked if any of these also need
> the flag, by any chance, so we could fix this once and for all?
>
> $ git grep -l 'target bpf ' | egrep -v '(Documentation|bpftool)'
> kernel/bpf/preload/iterators/Makefile
> samples/bpf/Makefile
> samples/bpf/test_lwt_bpf.sh
> tools/bpf/runqslower/Makefile
> tools/build/feature/Makefile
> tools/perf/Makefile.perf
> tools/perf/util/llvm-utils.c
> tools/testing/selftests/bpf/Makefile
> tools/testing/selftests/net/bpf/Makefile
> tools/testing/selftests/tc-testing/Makefile
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?
On Mon, 2023-01-16 at 14:49 -0800, Peter Foley wrote:
> On Mon, Jan 16, 2023 at 4:59 AM Eduard Zingerman <[email protected]> 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.
Ok, thanks for the input, I'll see what can be done.
> But given that won't help already released versions of clang, it
> should probably happen in addition to this patch.
Yes, of-course.
On Mon, Jan 16, 2023 at 4:59 AM Eduard Zingerman <[email protected]> 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.
On 1/16/23 2:49 PM, Peter Foley wrote:
> On Mon, Jan 16, 2023 at 4:59 AM Eduard Zingerman <[email protected]> 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?
On Mon, Jan 16, 2023 at 11:05 PM Yonghong Song <[email protected]> wrote:
>
> 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?
>
The relevant override appears to be
https://github.com/gentoo/gentoo/blob/c5247250e9d4a09e67a602965a5f72be3cebbf34/sys-devel/clang-common/clang-common-15.0.7.ebuild#L93
> On 1/16/23 2:49 PM, Peter Foley wrote:
>> On Mon, Jan 16, 2023 at 4:59 AM Eduard Zingerman <[email protected]> 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.
On 1/16/23 11:09 PM, Peter Foley wrote:
> On Mon, Jan 16, 2023 at 11:05 PM Yonghong Song <[email protected]> wrote:
>>
>> 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?
>>
> The relevant override appears to be
> https://github.com/gentoo/gentoo/blob/c5247250e9d4a09e67a602965a5f72be3cebbf34/sys-devel/clang-common/clang-common-15.0.7.ebuild#L93
Thanks for the link. Looks like this is a security feature added by
hardened_gentoo progject (https://wiki.gentoo.org/wiki/Hardened_Gentoo)
which unconditionally added -fstack-protector-strong to the clang
compilation flag.
>> On 1/16/23 2:49 PM, Peter Foley wrote:
>>> On Mon, Jan 16, 2023 at 4:59 AM Eduard Zingerman <[email protected]> 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;
+ }
}
#undef TARGET_OPTION_OVERRIDE
--
2.30.2
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 <[email protected]> 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
On 1/17/23 5:23 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 <[email protected]> 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.
clang itself does not have -fstack-protector on by default. It is
hardened gentoo distribution unconditionally added -fstack-protector
to its clang distribution.
In clang/lib/Driver/ToolChains/Clang.cpp, we have
...
// NVPTX doesn't support stack protectors; from the compiler's
perspective, it
// doesn't even have a stack!
if (EffectiveTriple.isNVPTX())
return;
and -fstack-protector is not effective for NVPTX. I guess we
could make it noop for BPF target as well.
On Sat, 2023-01-14 at 18:00 -0500, Peter Foley wrote:
> Avoid build errors on distros that force the stack protector on by
> default.
> e.g.
> CLANG /home/peter/linux/work/tools/bpf/bpftool/pid_iter.bpf.o
> skeleton/pid_iter.bpf.c:53:5: error: A call to built-in function '__stack_chk_fail' is not supported.
> int iter(struct bpf_iter__task_file *ctx)
> ^
> 1 error generated.
>
> Signed-off-by: Peter Foley <[email protected]>
> ---
> tools/bpf/bpftool/Makefile | 1 +
> tools/bpf/runqslower/Makefile | 5 +++--
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> index f610e184ce02a..36ac0002e386f 100644
> --- a/tools/bpf/bpftool/Makefile
> +++ b/tools/bpf/bpftool/Makefile
> @@ -215,6 +215,7 @@ $(OUTPUT)%.bpf.o: skeleton/%.bpf.c $(OUTPUT)vmlinux.h $(LIBBPF_BOOTSTRAP)
> -I$(or $(OUTPUT),.) \
> -I$(srctree)/tools/include/uapi/ \
> -I$(LIBBPF_BOOTSTRAP_INCLUDE) \
> + -fno-stack-protector \
While working on clang patch to disable stack protector
for BPF target I've noticed that there is an option to
disable default configuration file altogether [1]:
--no-default-config
Should we consider it instead of -fno-stack-protector
to shield ourselves from any potential distro-specific
changes?
[1] https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-no-default-config
> -g -O2 -Wall -target bpf -c $< -o $@
> $(Q)$(LLVM_STRIP) -g $@
>
> diff --git a/tools/bpf/runqslower/Makefile b/tools/bpf/runqslower/Makefile
> index 8b3d87b82b7a2..f7313cc966a04 100644
> --- a/tools/bpf/runqslower/Makefile
> +++ b/tools/bpf/runqslower/Makefile
> @@ -60,8 +60,9 @@ $(OUTPUT)/%.skel.h: $(OUTPUT)/%.bpf.o | $(BPFTOOL)
> $(QUIET_GEN)$(BPFTOOL) gen skeleton $< > $@
>
> $(OUTPUT)/%.bpf.o: %.bpf.c $(BPFOBJ) | $(OUTPUT)
> - $(QUIET_GEN)$(CLANG) -g -O2 -target bpf $(INCLUDES) \
> - -c $(filter %.c,$^) -o $@ && \
> + $(QUIET_GEN)$(CLANG) -g -O2 -target bpf $(INCLUDES) \
> + -fno-stack-protector \
> + -c $(filter %.c,$^) -o $@ && \
> $(LLVM_STRIP) -g $@
>
> $(OUTPUT)/%.o: %.c | $(OUTPUT)
>
> ---
> base-commit: 97ec4d559d939743e8af83628be5af8da610d9dc
> change-id: 20230114-bpf-918ae127b77a
>
> Best regards,
On 1/18/23 11:28 AM, Eduard Zingerman wrote:
> On Sat, 2023-01-14 at 18:00 -0500, Peter Foley wrote:
>> Avoid build errors on distros that force the stack protector on by
>> default.
>> e.g.
>> CLANG /home/peter/linux/work/tools/bpf/bpftool/pid_iter.bpf.o
>> skeleton/pid_iter.bpf.c:53:5: error: A call to built-in function '__stack_chk_fail' is not supported.
>> int iter(struct bpf_iter__task_file *ctx)
>> ^
>> 1 error generated.
>>
>> Signed-off-by: Peter Foley <[email protected]>
>> ---
>> tools/bpf/bpftool/Makefile | 1 +
>> tools/bpf/runqslower/Makefile | 5 +++--
>> 2 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
>> index f610e184ce02a..36ac0002e386f 100644
>> --- a/tools/bpf/bpftool/Makefile
>> +++ b/tools/bpf/bpftool/Makefile
>> @@ -215,6 +215,7 @@ $(OUTPUT)%.bpf.o: skeleton/%.bpf.c $(OUTPUT)vmlinux.h $(LIBBPF_BOOTSTRAP)
>> -I$(or $(OUTPUT),.) \
>> -I$(srctree)/tools/include/uapi/ \
>> -I$(LIBBPF_BOOTSTRAP_INCLUDE) \
>> + -fno-stack-protector \
>
> While working on clang patch to disable stack protector
> for BPF target I've noticed that there is an option to
> disable default configuration file altogether [1]:
>
> --no-default-config
>
> Should we consider it instead of -fno-stack-protector
> to shield ourselves from any potential distro-specific
> changes?
Peter, could you help check whether adding --no-default-config works
in your environment or not?
>
> [1] https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-no-default-config
>
>> -g -O2 -Wall -target bpf -c $< -o $@
>> $(Q)$(LLVM_STRIP) -g $@
>>
>> diff --git a/tools/bpf/runqslower/Makefile b/tools/bpf/runqslower/Makefile
>> index 8b3d87b82b7a2..f7313cc966a04 100644
>> --- a/tools/bpf/runqslower/Makefile
>> +++ b/tools/bpf/runqslower/Makefile
>> @@ -60,8 +60,9 @@ $(OUTPUT)/%.skel.h: $(OUTPUT)/%.bpf.o | $(BPFTOOL)
>> $(QUIET_GEN)$(BPFTOOL) gen skeleton $< > $@
>>
>> $(OUTPUT)/%.bpf.o: %.bpf.c $(BPFOBJ) | $(OUTPUT)
>> - $(QUIET_GEN)$(CLANG) -g -O2 -target bpf $(INCLUDES) \
>> - -c $(filter %.c,$^) -o $@ && \
>> + $(QUIET_GEN)$(CLANG) -g -O2 -target bpf $(INCLUDES) \
>> + -fno-stack-protector \
>> + -c $(filter %.c,$^) -o $@ && \
>> $(LLVM_STRIP) -g $@
>>
>> $(OUTPUT)/%.o: %.c | $(OUTPUT)
>>
>> ---
>> base-commit: 97ec4d559d939743e8af83628be5af8da610d9dc
>> change-id: 20230114-bpf-918ae127b77a
>>
>> Best regards,
>
On Wed, Jan 18, 2023 at 11:34 PM Yonghong Song <[email protected]> wrote:
> On 1/18/23 11:28 AM, Eduard Zingerman wrote:
> >
> > While working on clang patch to disable stack protector
> > for BPF target I've noticed that there is an option to
> > disable default configuration file altogether [1]:
> >
> > --no-default-config
> >
> > Should we consider it instead of -fno-stack-protector
> > to shield ourselves from any potential distro-specific
> > changes?
>
> Peter, could you help check whether adding --no-default-config works
> in your environment or not?
>
> >
> > [1] https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-no-default-config
I guess I could, but I'm not convinced that's the right thing to do.
Ideally problems with distro-specific configs would cause loud
failures (like this one) and result in fixes like the changes being
made to upstream clang/gcc.
Simply unconditionally disabling distro configs seems to be the wrong
way to approach this and makes it less likely that future problems
will be reported in the first place.
On 1/22/23 8:28 PM, Peter Foley wrote:
> On Wed, Jan 18, 2023 at 11:34 PM Yonghong Song <[email protected]> wrote:
>> On 1/18/23 11:28 AM, Eduard Zingerman wrote:
>>>
>>> While working on clang patch to disable stack protector
>>> for BPF target I've noticed that there is an option to
>>> disable default configuration file altogether [1]:
>>>
>>> --no-default-config
>>>
>>> Should we consider it instead of -fno-stack-protector
>>> to shield ourselves from any potential distro-specific
>>> changes?
>>
>> Peter, could you help check whether adding --no-default-config works
>> in your environment or not?
>>
>>>
>>> [1] https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-no-default-config
>
> I guess I could, but I'm not convinced that's the right thing to do.
> Ideally problems with distro-specific configs would cause loud
> failures (like this one) and result in fixes like the changes being
> made to upstream clang/gcc.
> Simply unconditionally disabling distro configs seems to be the wrong
> way to approach this and makes it less likely that future problems
> will be reported in the first place.
Thanks for confirming. Eduard has implemented a proper fix in clang
(https://reviews.llvm.org/D142046) which will warn if -fstack-protector
is enabled. In gentoo case, since -fno-stack-protector is appended to
compilation flags, no warning will be issued, so we are all good here.