Re: [PATCH 1/1] perf bpf_skel augmented_raw_syscalls: Cap the socklen parameter using &= sizeof(saddr)

[Arnaldo Carvalho de Melo]

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@xxxxxxxxxx> 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):

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]$