```
commit f89231e503dc0b97c81f4bb32d28532fb3471acd
Author: Bastien Curutchet (eBPF Foundation) <[email protected]>
selftests/bpf: test_xsk: Don't exit immediately when workers fail
This commit converts the test program from using exit_with_error()
(which immediately terminates) to proper error propagation through
return values. This allows the test suite to continue running after
failures.
> diff --git a/tools/testing/selftests/bpf/test_xsk.c
> b/tools/testing/selftests/bpf/test_xsk.c
> index 2c955e109..2c392d5b9 100644
> --- a/tools/testing/selftests/bpf/test_xsk.c
> +++ b/tools/testing/selftests/bpf/test_xsk.c
[ ... ]
> @@ -759,7 +761,7 @@ static bool is_metadata_correct(struct pkt *pkt, void
> *buffer, u64 addr)
> return true;
> }
>
> -static bool is_adjust_tail_supported(struct xsk_xdp_progs *skel_rx)
> +static int is_adjust_tail_supported(struct xsk_xdp_progs *skel_rx, bool
> *supported)
> {
> struct bpf_map *data_map;
> int adjust_value = 0;
> @@ -769,19 +771,21 @@ static bool is_adjust_tail_supported(struct
> xsk_xdp_progs *skel_rx)
> data_map = bpf_object__find_map_by_name(skel_rx->obj, "xsk_xdp_.bss");
> if (!data_map || !bpf_map__is_internal(data_map)) {
> ksft_print_msg("Error: could not find bss section of XDP
> program\n");
> - exit_with_error(errno);
> + return -EINVAL;
> }
>
> ret = bpf_map_lookup_elem(bpf_map__fd(data_map), &key, &adjust_value);
> if (ret) {
> ksft_print_msg("Error: bpf_map_lookup_elem failed with error
> %d\n", ret);
> - exit_with_error(errno);
> + return ret;
> }
>
> /* Set the 'adjust_value' variable to -EOPNOTSUPP in the XDP program if
> the adjust_tail
> * helper is not supported. Skip the adjust_tail test case in this
> scenario.
> */
> - return adjust_value != -EOPNOTSUPP;
> + *supported = adjust_value != -EOPNOTSUPP;
> +
> + return 0;
> }
[ ... ]
> @@ -1584,29 +1614,41 @@ void *worker_testapp_validate_rx(void *arg)
> int err;
>
> if (test->current_step == 1) {
> - thread_common_ops(test, ifobject);
> + err = thread_common_ops(test, ifobject);
> } else {
> xsk_clear_xskmap(ifobject->xskmap);
> err = xsk_update_xskmap(ifobject->xskmap, ifobject->xsk->xsk,
> 0);
> - if (err) {
> + if (err)
> ksft_print_msg("Error: Failed to update xskmap, error
> %s\n",
> strerror(-err));
> - exit_with_error(-err);
> - }
> }
>
> pthread_barrier_wait(&barr);
>
> + /* We leave only now in case of error to avoid getting stuck in the
> barrier */
> + if (err) {
> + test->fail = true;
> + pthread_exit(NULL);
> + }
> +
> err = receive_pkts(test);
>
> if (!err && ifobject->validation_func)
> err = ifobject->validation_func(ifobject);
>
> if (err) {
> - if (test->adjust_tail &&
> !is_adjust_tail_supported(ifobject->xdp_progs))
> - test->adjust_tail_support = false;
> - else
> + if (!test->adjust_tail) {
> test->fail = true;
> + } else {
> + bool supported;
> +
> + if (is_adjust_tail_supported(ifobject->xdp_progs,
> &supported))
> + test->fail = true;
> + if (!supported)
> + test->adjust_tail_support = false;
> + else
> + test->fail = true;
> + }
> }
Can the 'supported' variable be read uninitialized here? When
is_adjust_tail_supported() returns an error (non-zero), it doesn't
write to the *supported output parameter. The function returns early
from either the data_map check or bpf_map_lookup_elem() failure without
setting *supported. Then the code unconditionally checks "if (!supported)"
which would read uninitialized stack memory.
The control flow is:
is_adjust_tail_supported() returns error
-> test->fail = true is set
-> execution continues to "if (!supported)"
-> reads uninitialized 'supported' variable
Should this be structured with an else clause to only check 'supported'
when is_adjust_tail_supported() returns success?
```
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
In-Reply-To-Subject: `selftests/bpf: test_xsk: Don't exit immediately when
workers fail`
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/18910697393
