On Thu, May 19, 2022 at 7:26 AM Alan Maguire <alan.maguire@xxxxxxxxxx> wrote:
> +void test_unpriv_bpf_disabled(void)
> +{
> + char *map_paths[NUM_MAPS] = { PINPATH "array",
> + PINPATH "percpu_array",
> + PINPATH "hash",
> + PINPATH "percpu_hash",
> + PINPATH "perfbuf",
> + PINPATH "ringbuf",
> + PINPATH "prog_array" };
> + int map_fds[NUM_MAPS];
> + struct test_unpriv_bpf_disabled *skel;
> + char unprivileged_bpf_disabled_orig[32] = {};
> + char perf_event_paranoid_orig[32] = {};
> + struct bpf_prog_info prog_info = {};
> + __u32 prog_info_len = sizeof(prog_info);
> + struct perf_event_attr attr = {};
> + int prog_fd, perf_fd, i, ret;
> + __u64 save_caps = 0;
> + __u32 prog_id;
> +
> + skel = test_unpriv_bpf_disabled__open_and_load();
> + if (!ASSERT_OK_PTR(skel, "skel_open"))
> + return;
> +
> + skel->bss->test_pid = getpid();
> +
> + map_fds[0] = bpf_map__fd(skel->maps.array);
> + map_fds[1] = bpf_map__fd(skel->maps.percpu_array);
> + map_fds[2] = bpf_map__fd(skel->maps.hash);
> + map_fds[3] = bpf_map__fd(skel->maps.percpu_hash);
> + map_fds[4] = bpf_map__fd(skel->maps.perfbuf);
> + map_fds[5] = bpf_map__fd(skel->maps.ringbuf);
> + map_fds[6] = bpf_map__fd(skel->maps.prog_array);
> +
> + for (i = 0; i < NUM_MAPS; i++)
> + ASSERT_OK(bpf_obj_pin(map_fds[i], map_paths[i]), "pin map_fd");
> +
> + /* allow user without caps to use perf events */
> + if (!ASSERT_OK(sysctl_set("/proc/sys/kernel/perf_event_paranoid", perf_event_paranoid_orig,
> + "-1"),
> + "set_perf_event_paranoid"))
> + goto cleanup;
> + /* ensure unprivileged bpf disabled is set */
> + ret = sysctl_set("/proc/sys/kernel/unprivileged_bpf_disabled",
> + unprivileged_bpf_disabled_orig, "2");
> + if (ret == -EPERM) {
> + /* if unprivileged_bpf_disabled=1, we get -EPERM back; that's okay. */
> + if (!ASSERT_OK(strcmp(unprivileged_bpf_disabled_orig, "1"),
> + "unpriviliged_bpf_disabled_on"))
> + goto cleanup;
> + } else {
> + if (!ASSERT_OK(ret, "set unpriviliged_bpf_disabled"))
> + goto cleanup;
> + }
Alan,
same as in v3 the BPF CI complained when selftests are built with clang.
/tools/testing/selftests/bpf/prog_tests/unpriv_bpf_disabled.c:267:7:
error: variable 'perf_fd' is used uninitialized whenever 'if'
condition is true [-Werror,-Wsometimes-uninitialized]
if (!ASSERT_OK(ret, "set unpriviliged_bpf_disabled"))
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tools/testing/selftests/bpf/prog_tests/unpriv_bpf_disabled.c:301:8:
note: uninitialized use occurs here
close(perf_fd);
^~~~~~~
/tools/testing/selftests/bpf/prog_tests/unpriv_bpf_disabled.c:267:3:
note: remove the 'if' if its condition is always false
if (!ASSERT_OK(ret, "set unpriviliged_bpf_disabled"))
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Looks like clang found the real issue.
I've addressed with:
- int prog_fd, perf_fd, i, ret;
+ int prog_fd, perf_fd = -1, i, ret;
while applying.
Please pay attention to CI errors.
To repro do: make LLVM=1 -j
