Message ID | 20230927045030.224548-2-ruowenq2@illinois.edu (mailing list archive) |
---|---|
State | Accepted |
Commit | 9e09b75079e229b08f12a732712100fdb9af8cab |
Delegated to: | BPF |
Headers | show |
Series | samples/bpf: syscall_tp_user: Refactor and fix array index out-of-bounds bug | expand |
On Tue, Sep 26, 2023 at 11:50:30PM -0500, ruowenq2@illinois.edu wrote: > From: Ruowen Qin <ruowenq2@illinois.edu> > > The sanitizer flag, which is supported by both clang and gcc, would make > it easier to debug array index out-of-bounds problems in these programs. > > Make the Makfile smarter to detect ubsan support from the compiler and > add the '-fsanitize=bounds' accordingly. > > Suggested-by: Mimi Zohar <zohar@linux.ibm.com> > Signed-off-by: Jinghao Jia <jinghao@linux.ibm.com> > Signed-off-by: Jinghao Jia <jinghao7@illinois.edu> > Signed-off-by: Ruowen Qin <ruowenq2@illinois.edu> > --- > samples/bpf/Makefile | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile > index 6c707ebcebb9..90af76fa9dd8 100644 > --- a/samples/bpf/Makefile > +++ b/samples/bpf/Makefile > @@ -169,6 +169,9 @@ endif > TPROGS_CFLAGS += -Wall -O2 > TPROGS_CFLAGS += -Wmissing-prototypes > TPROGS_CFLAGS += -Wstrict-prototypes > +TPROGS_CFLAGS += $(call try-run,\ > + printf "int main() { return 0; }" |\ > + $(CC) -Werror -fsanitize=bounds -x c - -o "$$TMP",-fsanitize=bounds,) I haven't checked deeply, but could we use just cc-option? looks simpler TPROGS_CFLAGS += $(call cc-option, -fsanitize=bounds) jirka > > TPROGS_CFLAGS += -I$(objtree)/usr/include > TPROGS_CFLAGS += -I$(srctree)/tools/testing/selftests/bpf/ > -- > 2.42.0 > >
On 9/27/23 6:03 AM, Jiri Olsa <olsajiri@gmail.com> wrote: > On Tue, Sep 26, 2023 at 11:50:30PM -0500, ruowenq2@illinois.edu wrote: > > From: Ruowen Qin <ruowenq2@illinois.edu> > > > > The sanitizer flag, which is supported by both clang and gcc, would make > > it easier to debug array index out-of-bounds problems in these programs. > > > > Make the Makfile smarter to detect ubsan support from the compiler and > > add the '-fsanitize=bounds' accordingly. > > > > Suggested-by: Mimi Zohar <zohar@linux.ibm.com> > > Signed-off-by: Jinghao Jia <jinghao@linux.ibm.com> > > Signed-off-by: Jinghao Jia <jinghao7@illinois.edu> > > Signed-off-by: Ruowen Qin <ruowenq2@illinois.edu> > > --- > > samples/bpf/Makefile | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile > > index 6c707ebcebb9..90af76fa9dd8 100644 > > --- a/samples/bpf/Makefile > > +++ b/samples/bpf/Makefile > > @@ -169,6 +169,9 @@ endif > > TPROGS_CFLAGS += -Wall -O2 > > TPROGS_CFLAGS += -Wmissing-prototypes > > TPROGS_CFLAGS += -Wstrict-prototypes > > +TPROGS_CFLAGS += $(call try-run,\ > > + printf "int main() { return 0; }" |\ > > + $(CC) -Werror -fsanitize=bounds -x c - -o "$$TMP",-fsanitize=bounds,) > > I haven't checked deeply, but could we use just cc-option? looks simpler > > TPROGS_CFLAGS += $(call cc-option, -fsanitize=bounds) > > jirka Hi, thanks for your quick reply! When checking for flags, cc-option does not execute the linker, but on Fedora, an error appears and stating that "/usr/lib64/libubsan.so.1.0.0" cannot be found during linking. So I try this seemingly cumbersome way. Ruowen > > > > TPROGS_CFLAGS += -I$(objtree)/usr/include > > TPROGS_CFLAGS += -I$(srctree)/tools/testing/selftests/bpf/ > > -- > > 2.42.0 > > > > >
On Wed, Sep 27, 2023 at 06:19:10PM -0500, ruowenq2@illinois.edu wrote: > > > On 9/27/23 6:03 AM, Jiri Olsa <olsajiri@gmail.com> wrote: > > On Tue, Sep 26, 2023 at 11:50:30PM -0500, ruowenq2@illinois.edu wrote: > > > From: Ruowen Qin <ruowenq2@illinois.edu> > > > > > > The sanitizer flag, which is supported by both clang and gcc, would make > > > it easier to debug array index out-of-bounds problems in these programs. > > > > > > Make the Makfile smarter to detect ubsan support from the compiler and > > > add the '-fsanitize=bounds' accordingly. > > > > > > Suggested-by: Mimi Zohar <zohar@linux.ibm.com> > > > Signed-off-by: Jinghao Jia <jinghao@linux.ibm.com> > > > Signed-off-by: Jinghao Jia <jinghao7@illinois.edu> > > > Signed-off-by: Ruowen Qin <ruowenq2@illinois.edu> > > > --- > > > samples/bpf/Makefile | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile > > > index 6c707ebcebb9..90af76fa9dd8 100644 > > > --- a/samples/bpf/Makefile > > > +++ b/samples/bpf/Makefile > > > @@ -169,6 +169,9 @@ endif > > > TPROGS_CFLAGS += -Wall -O2 > > > TPROGS_CFLAGS += -Wmissing-prototypes > > > TPROGS_CFLAGS += -Wstrict-prototypes > > > +TPROGS_CFLAGS += $(call try-run,\ > > > + printf "int main() { return 0; }" |\ > > > + $(CC) -Werror -fsanitize=bounds -x c - -o "$$TMP",-fsanitize=bounds,) > > > > I haven't checked deeply, but could we use just cc-option? looks simpler > > > > TPROGS_CFLAGS += $(call cc-option, -fsanitize=bounds) > > > > jirka > > Hi, thanks for your quick reply! When checking for flags, cc-option does not execute the linker, but on Fedora, an error appears and stating that "/usr/lib64/libubsan.so.1.0.0" cannot be found during linking. So I try this seemingly cumbersome way. I see, there's also ld-option, would that work? jirka > > Ruowen > > > > > TPROGS_CFLAGS += -I$(objtree)/usr/include > > > TPROGS_CFLAGS += -I$(srctree)/tools/testing/selftests/bpf/ > > > -- > 2.42.0 > > > > > > > >
On 9/28/23 3:15 AM, Jiri Olsa wrote: > On Wed, Sep 27, 2023 at 06:19:10PM -0500, ruowenq2@illinois.edu wrote: >> >> >> On 9/27/23 6:03 AM, Jiri Olsa <olsajiri@gmail.com> wrote: >>> On Tue, Sep 26, 2023 at 11:50:30PM -0500, ruowenq2@illinois.edu wrote: >>>> From: Ruowen Qin <ruowenq2@illinois.edu> >>>> >>>> The sanitizer flag, which is supported by both clang and gcc, would make >>>> it easier to debug array index out-of-bounds problems in these programs. >>>> >>>> Make the Makfile smarter to detect ubsan support from the compiler and >>>> add the '-fsanitize=bounds' accordingly. >>>> >>>> Suggested-by: Mimi Zohar <zohar@linux.ibm.com> >>>> Signed-off-by: Jinghao Jia <jinghao@linux.ibm.com> >>>> Signed-off-by: Jinghao Jia <jinghao7@illinois.edu> >>>> Signed-off-by: Ruowen Qin <ruowenq2@illinois.edu> >>>> --- >>>> samples/bpf/Makefile | 3 +++ >>>> 1 file changed, 3 insertions(+) >>>> >>>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile >>>> index 6c707ebcebb9..90af76fa9dd8 100644 >>>> --- a/samples/bpf/Makefile >>>> +++ b/samples/bpf/Makefile >>>> @@ -169,6 +169,9 @@ endif >>>> TPROGS_CFLAGS += -Wall -O2 >>>> TPROGS_CFLAGS += -Wmissing-prototypes >>>> TPROGS_CFLAGS += -Wstrict-prototypes >>>> +TPROGS_CFLAGS += $(call try-run,\ >>>> + printf "int main() { return 0; }" |\ >>>> + $(CC) -Werror -fsanitize=bounds -x c - -o "$$TMP",-fsanitize=bounds,) >>> >>> I haven't checked deeply, but could we use just cc-option? looks simpler >>> >>> TPROGS_CFLAGS += $(call cc-option, -fsanitize=bounds) >>> >>> jirka >> >> Hi, thanks for your quick reply! When checking for flags, cc-option does not execute the linker, but on Fedora, an error appears and stating that "/usr/lib64/libubsan.so.1.0.0" cannot be found during linking. So I try this seemingly cumbersome way. > > I see, there's also ld-option, would that work? > > jirka > IMHO I don't think ld-option would solve the problem. It directly sends the flag to the linker but -fsanitize=bounds is a compiler flag, not a linker flag. Basically, what's special about this case is that the feature we want to probe is behind a gcc/clang flag but we do not know whether it is supported until link time (e.g. the sanitizer library is missing on Fedora so we get a link error). --Jinghao >> >> Ruowen >> >>>> > TPROGS_CFLAGS += -I$(objtree)/usr/include >>>> TPROGS_CFLAGS += -I$(srctree)/tools/testing/selftests/bpf/ >>>> -- > 2.42.0 >>>> >>>> >>>
On Thu, Sep 28, 2023 at 04:19:02AM -0500, Jinghao Jia wrote: > > > On 9/28/23 3:15 AM, Jiri Olsa wrote: > > On Wed, Sep 27, 2023 at 06:19:10PM -0500, ruowenq2@illinois.edu wrote: > >> > >> > >> On 9/27/23 6:03 AM, Jiri Olsa <olsajiri@gmail.com> wrote: > >>> On Tue, Sep 26, 2023 at 11:50:30PM -0500, ruowenq2@illinois.edu wrote: > >>>> From: Ruowen Qin <ruowenq2@illinois.edu> > >>>> > >>>> The sanitizer flag, which is supported by both clang and gcc, would make > >>>> it easier to debug array index out-of-bounds problems in these programs. > >>>> > >>>> Make the Makfile smarter to detect ubsan support from the compiler and > >>>> add the '-fsanitize=bounds' accordingly. > >>>> > >>>> Suggested-by: Mimi Zohar <zohar@linux.ibm.com> > >>>> Signed-off-by: Jinghao Jia <jinghao@linux.ibm.com> > >>>> Signed-off-by: Jinghao Jia <jinghao7@illinois.edu> > >>>> Signed-off-by: Ruowen Qin <ruowenq2@illinois.edu> > >>>> --- > >>>> samples/bpf/Makefile | 3 +++ > >>>> 1 file changed, 3 insertions(+) > >>>> > >>>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile > >>>> index 6c707ebcebb9..90af76fa9dd8 100644 > >>>> --- a/samples/bpf/Makefile > >>>> +++ b/samples/bpf/Makefile > >>>> @@ -169,6 +169,9 @@ endif > >>>> TPROGS_CFLAGS += -Wall -O2 > >>>> TPROGS_CFLAGS += -Wmissing-prototypes > >>>> TPROGS_CFLAGS += -Wstrict-prototypes > >>>> +TPROGS_CFLAGS += $(call try-run,\ > >>>> + printf "int main() { return 0; }" |\ > >>>> + $(CC) -Werror -fsanitize=bounds -x c - -o "$$TMP",-fsanitize=bounds,) > >>> > >>> I haven't checked deeply, but could we use just cc-option? looks simpler > >>> > >>> TPROGS_CFLAGS += $(call cc-option, -fsanitize=bounds) > >>> > >>> jirka > >> > >> Hi, thanks for your quick reply! When checking for flags, cc-option does not execute the linker, but on Fedora, an error appears and stating that "/usr/lib64/libubsan.so.1.0.0" cannot be found during linking. So I try this seemingly cumbersome way. > > > > I see, there's also ld-option, would that work? > > > > jirka > > > > IMHO I don't think ld-option would solve the problem. It directly sends the > flag to the linker but -fsanitize=bounds is a compiler flag, not a linker > flag. > > Basically, what's special about this case is that the feature we want to > probe is behind a gcc/clang flag but we do not know whether it is supported > until link time (e.g. the sanitizer library is missing on Fedora so we get > a link error). ok, I tested on fedora, looks good Acked-by: Jiri Olsa <jolsa@kernel.org> Tested-by: Jiri Olsa <jolsa@kernel.org> thanks, jirka > > --Jinghao > > >> > >> Ruowen > >> > >>>> > TPROGS_CFLAGS += -I$(objtree)/usr/include > >>>> TPROGS_CFLAGS += -I$(srctree)/tools/testing/selftests/bpf/ > >>>> -- > 2.42.0 > >>>> > >>>> > >>>
diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile index 6c707ebcebb9..90af76fa9dd8 100644 --- a/samples/bpf/Makefile +++ b/samples/bpf/Makefile @@ -169,6 +169,9 @@ endif TPROGS_CFLAGS += -Wall -O2 TPROGS_CFLAGS += -Wmissing-prototypes TPROGS_CFLAGS += -Wstrict-prototypes +TPROGS_CFLAGS += $(call try-run,\ + printf "int main() { return 0; }" |\ + $(CC) -Werror -fsanitize=bounds -x c - -o "$$TMP",-fsanitize=bounds,) TPROGS_CFLAGS += -I$(objtree)/usr/include TPROGS_CFLAGS += -I$(srctree)/tools/testing/selftests/bpf/