On Fri, Jan 23, 2026 at 5:50 PM <[email protected]> wrote:
>
> > diff --git a/tools/testing/selftests/bpf/prog_tests/wakeup_source_iter.c 
> > b/tools/testing/selftests/bpf/prog_tests/wakeup_source_iter.c
> > new file mode 100644
> > index 000000000000..c8a38717e284
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/prog_tests/wakeup_source_iter.c
>
> [ ... ]
>
> > +static int get_ws_iter_stream(struct wakeup_source_iter *skel, int 
> > *iter_fd,
> > +                           FILE **iter_file)
> > +{
> > +     *iter_fd = bpf_iter_create(
> > +                     bpf_link__fd(skel->links.wakeup_source_collector));
> > +     if (!ASSERT_OK_FD(*iter_fd, "iter_create"))
> > +             return -1;
> > +
> > +     *iter_file = fdopen(*iter_fd, "r");
> > +     if (!ASSERT_OK_PTR(*iter_file, "fdopen"))
> > +             return -1;
> > +
> > +     return 0;
> > +}
>
> If bpf_iter_create() fails above, get_ws_iter_stream() returns -1 without
> writing to *iter_file.
>

This should be fine, if the BPF iter file isn't created, we will not
write to that file.

> > +static void subtest_ws_iter_check_active_count(struct wakeup_source_iter 
> > *skel)
> > +{
>
> [ ... ]
>
> > +     char *line = NULL;
> > +     bool found_ws = false;
> > +     FILE *iter_file = NULL;
> > +     int iter_fd = -1;
>
> This function correctly initializes iter_file and iter_fd before calling
> get_ws_iter_stream().
>
> [ ... ]
>
> > +}
> > +
> > +static void subtest_ws_iter_check_sleep_times(struct wakeup_source_iter 
> > *skel)
> > +{
> > +     bool found_test_ws = false;
> > +     struct WakeupSourceInfo ws_info;
> > +     char *line = NULL;
> > +     FILE *iter_file;
> > +     int iter_fd;
>
> In contrast to subtest_ws_iter_check_active_count(), iter_file and iter_fd
> are not initialized here.
>

Ack, this is important and I can address it in the v4 of the patch.

> > +
> > +     if (get_ws_iter_stream(skel, &iter_fd, &iter_file))
> > +             goto cleanup;
>
> [ ... ]
>
> > +cleanup:
> > +     if (iter_file)
> > +             fclose(iter_file);
> > +     else if (iter_fd >= 0)
> > +             close(iter_fd);
> > +}
>
> If get_ws_iter_stream() fails at bpf_iter_create(), can this access
> uninitialized iter_file and iter_fd? The cleanup code would then evaluate
> garbage values, potentially calling fclose() with an invalid pointer.
>

Correct, and this issue will be fixed when the variables are
initialized as pointed out in the previous comment.

>
> ---
> 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
>
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/21306880863

Reply via email to