Messages in this thread | | | From | Ian Rogers <> | Date | Tue, 22 Aug 2023 11:16:44 -0700 | Subject | Re: [PATCH 1/1] perf bpf_skel augmented_raw_syscalls: Cap the socklen parameter using &= sizeof(saddr) |
| |
On Tue, Aug 22, 2023 at 11:08 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > > Em Mon, Aug 21, 2023 at 07:52:46AM -0700, Ian Rogers escreveu: > > On Mon, Aug 21, 2023 at 6:40 AM Arnaldo Carvalho de Melo > > <acme@kernel.org> wrote: > > > > > > Em Wed, Aug 16, 2023 at 03:10:00PM -0700, Ian Rogers escreveu: > > > > On Wed, Aug 16, 2023 at 2:48 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > > > > > R2 min value is negative, either use unsigned or 'var &= const' > > > > > processed 22 insns (limit 1000000) max_states_per_insn 0 total_states 1 peak_states 1 mark_read 1 > > > > > -- END PROG LOAD LOG -- > > > > > libbpf: prog 'sys_enter_sendto': failed to load: -13 > > > > > libbpf: failed to load object 'augmented_raw_syscalls_bpf' > > > > > libbpf: failed to load BPF skeleton 'augmented_raw_syscalls_bpf': -13 > > > > > > > > > > So use the suggested &= variant since sizeof(saddr) == 128 bytes. > > > > > > > > Could this be an assert? > > > > > > you mean (removing the change to saddr to make it trigger): > > > > I was more meaning that doing sizeof(x) -1 and assuming that x is a > > power of two should really have an assert that sizeof(x) is a power of > > Right, that was my understanding, and that is what: > > _Static_assert(is_power_of_2(sizeof(augmented_args->saddr)), "sizeof(augmented_args->saddr) needs to be a power of two") > > Does, right?
Yep, I was reading your emails in order :-)
Thanks, Ian
> - Arnaldo > > > > 2. Using 127 and asserting sizeof(x)==128 also works. > > > > Thanks, > > Ian > > > > > diff --git a/tools/perf/util/bpf_skel/augmented_raw_syscalls.bpf.c b/tools/perf/util/bpf_skel/augmented_raw_syscalls.bpf.c > > > index 9c1d0b271b20f693..521ce2d7357d983c 100644 > > > --- a/tools/perf/util/bpf_skel/augmented_raw_syscalls.bpf.c > > > +++ b/tools/perf/util/bpf_skel/augmented_raw_syscalls.bpf.c > > > @@ -10,6 +10,16 @@ > > > #include <bpf/bpf_helpers.h> > > > #include <linux/limits.h> > > > > > > +/** > > > + * is_power_of_2() - check if a value is a power of two > > > + * @n: the value to check > > > + * > > > + * Determine whether some value is a power of two, where zero is *not* > > > + * considered a power of two. Return: true if @n is a power of 2, otherwise > > > + * false. > > > + */ > > > +#define is_power_of_2(n) (n != 0 && ((n & (n - 1)) == 0)) > > > + > > > #define MAX_CPUS 4096 > > > > > > // FIXME: These should come from system headers > > > @@ -112,7 +122,10 @@ struct augmented_args_payload { > > > struct { > > > struct augmented_arg arg, arg2; > > > }; > > > - struct sockaddr_storage saddr; > > > + struct { > > > + struct sockaddr_storage real_saddr; > > > + char padding; > > > + } saddr; > > > char __data[sizeof(struct augmented_arg)]; > > > }; > > > }; > > > @@ -187,6 +200,7 @@ int sys_enter_connect(struct syscall_enter_args *args) > > > if (augmented_args == NULL) > > > return 1; /* Failure: don't filter */ > > > > > > + _Static_assert(is_power_of_2(sizeof(augmented_args->saddr)), "sizeof(augmented_args->saddr) needs to be a power of two"); > > > socklen &= sizeof(augmented_args->saddr) - 1; > > > > > > bpf_probe_read(&augmented_args->saddr, socklen, sockaddr_arg); > > > > > > > > > -------------------------------- > > > > > > CLANG /tmp/build/perf-tools-next/util/bpf_skel/.tmp/augmented_raw_syscalls.bpf.o > > > util/bpf_skel/augmented_raw_syscalls.bpf.c:203:2: error: static_assert failed due to requirement 'sizeof (augmented_args->saddr) != 0 && ((sizeof (augmented_args->saddr) & (sizeof (augmented_args->saddr) - 1)) == 0)' "sizeof(augmented_args->saddr) needs to be a power of two" > > > _Static_assert(is_power_of_2(sizeof(augmented_args->saddr)), "sizeof(augmented_args->saddr) needs to be a power of two"); > > > ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > 1 error generated. > > > make[2]: *** [Makefile.perf:1104: /tmp/build/perf-tools-next/util/bpf_skel/.tmp/augmented_raw_syscalls.bpf.o] Error 1 > > > make[1]: *** [Makefile.perf:238: sub-make] Error 2 > > > make: *** [Makefile:113: install-bin] Error 2 > > > make: Leaving directory '/var/home/acme/git/perf-tools-next/tools/perf' > > > > > > And here the assert being satisfied: > > > > > > ⬢[acme@toolbox perf-tools-next]$ git diff > > > diff --git a/tools/perf/util/bpf_skel/augmented_raw_syscalls.bpf.c b/tools/perf/util/bpf_skel/augmented_raw_syscalls.bpf.c > > > index 9c1d0b271b20f693..43549b63b433d81e 100644 > > > --- a/tools/perf/util/bpf_skel/augmented_raw_syscalls.bpf.c > > > +++ b/tools/perf/util/bpf_skel/augmented_raw_syscalls.bpf.c > > > @@ -10,6 +10,16 @@ > > > #include <bpf/bpf_helpers.h> > > > #include <linux/limits.h> > > > > > > +/** > > > + * is_power_of_2() - check if a value is a power of two > > > + * @n: the value to check > > > + * > > > + * Determine whether some value is a power of two, where zero is *not* > > > + * considered a power of two. Return: true if @n is a power of 2, otherwise > > > + * false. > > > + */ > > > +#define is_power_of_2(n) (n != 0 && ((n & (n - 1)) == 0)) > > > + > > > #define MAX_CPUS 4096 > > > > > > // FIXME: These should come from system headers > > > @@ -187,6 +197,7 @@ int sys_enter_connect(struct syscall_enter_args *args) > > > if (augmented_args == NULL) > > > return 1; /* Failure: don't filter */ > > > > > > + _Static_assert(is_power_of_2(sizeof(augmented_args->saddr)), "sizeof(augmented_args->saddr) needs to be a power of two"); > > > socklen &= sizeof(augmented_args->saddr) - 1; > > > > > > bpf_probe_read(&augmented_args->saddr, socklen, sockaddr_arg); > > > ⬢[acme@toolbox perf-tools-next]$ m > > > make: Entering directory '/var/home/acme/git/perf-tools-next/tools/perf' > > > BUILD: Doing 'make -j32' parallel build > > > INSTALL libsubcmd_headers > > > INSTALL libperf_headers > > > INSTALL libapi_headers > > > INSTALL libbpf_headers > > > CLANG /tmp/build/perf-tools-next/util/bpf_skel/.tmp/augmented_raw_syscalls.bpf.o > > > GENSKEL /tmp/build/perf-tools-next/util/bpf_skel/augmented_raw_syscalls.skel.h > > > INSTALL libsymbol_headers > > > CC /tmp/build/perf-tools-next/builtin-trace.o > > > LD /tmp/build/perf-tools-next/perf-in.o > > > LINK /tmp/build/perf-tools-next/perf > > > INSTALL binaries > > > INSTALL tests > > > INSTALL libperf-jvmti.so > > > INSTALL libexec > > > INSTALL perf-archive > > > INSTALL perf-iostat > > > INSTALL strace/groups > > > INSTALL perl-scripts > > > INSTALL python-scripts > > > INSTALL dlfilters > > > INSTALL perf_completion-script > > > INSTALL perf-tip > > > make: Leaving directory '/var/home/acme/git/perf-tools-next/tools/perf' > > > ⬢[acme@toolbox perf-tools-next]$ > > > > > > > > > > > > > > -- > > - Arnaldo
| |