Skip to content

Conversation

@kernel-patches-daemon-bpf
Copy link

Pull request for series with
subject: selftests/bpf: Fix failure path in send_signal test
version: 1
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=1022813

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 3249e8a
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1022813
version: 1

@kernel-patches-review-bot
Copy link

commit dfcd8cbada1cce92f24923a824935527506334ea
Author: Alexei Starovoitov <[email protected]>

selftests/bpf: Fix failure path in send_signal test

This patch fixes a test hang where the parent process closes pipes before
the child process exits, causing the child to enter an infinite loop and
the parent to hang in wait(NULL). The fix adds kill(pid, SIGKILL) before
the goto statement when skel open/load fails.

> diff --git a/tools/testing/selftests/bpf/prog_tests/send_signal.c b/tools/testing/selftests/bpf/prog_tests/send_signal.c
> index 1702aa592..61521dc76 100644
> --- a/tools/testing/selftests/bpf/prog_tests/send_signal.c
> +++ b/tools/testing/selftests/bpf/prog_tests/send_signal.c
> @@ -110,8 +110,10 @@ static void test_send_signal_common(struct perf_event_attr *attr,
>  	close(pipe_p2c[0]); /* close read */
>
>  	skel = test_send_signal_kern__open_and_load();
> -	if (!ASSERT_OK_PTR(skel, "skel_open_and_load"))
> +	if (!ASSERT_OK_PTR(skel, "skel_open_and_load")) {
> +		kill(pid, SIGKILL);
>  		goto skel_open_load_failure;
> +	}

The fix addresses the skel_open_and_load failure path, but does this same
issue affect the other early failure paths in test_send_signal_common()?

Looking at the code after this hunk:

	if (!attr) {
		err = test_send_signal_kern__attach(skel);
		if (!ASSERT_OK(err, "skel_attach")) {
			err = -1;
			goto destroy_skel;
		}
	} else {
		[ ... ]
		if (!ASSERT_GE(pmu_fd, 0, "perf_event_open")) {
			err = -1;
			goto destroy_skel;
		}

		skel->links.send_signal_perf =
			bpf_program__attach_perf_event(skel->progs.send_signal_perf, pmu_fd);
		if (!ASSERT_OK_PTR(skel->links.send_signal_perf, "attach_perf_event"))
			goto disable_pmu;
	}

	/* wait until child signal handler installed */
	ASSERT_EQ(read(pipe_c2p[0], buf, 1), 1, "pipe_read");

All three failure paths (attach, perf_event_open, attach_perf_event) occur
before reading from the child at pipe_c2p[0]. They goto labels that
eventually reach skel_open_load_failure without killing the child first.
Won't these paths trigger the same infinite loop bug described in the
commit message?



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: Fix failure path in send_signal test
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/19323536860

@kernel-patches-daemon-bpf
Copy link
Author

Forwarding comment 3526005172 via email
In-Reply-To: [email protected]
Patch: https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: f1d8c65
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1022813
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: f1d8c65
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1022813
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 93ce3be
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1022813
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 93ce3be
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1022813
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: c1da3df
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1022813
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: e5d2e34
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1022813
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: fea3f5e
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1022813
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 63066b7
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1022813
version: 1

When test_send_signal_kern__open_and_load() fails parent closes the
pipe which cases ASSERT_EQ(read(pipe_p2c...)) to fail, but child
continues and enters infinite loop, while parent is stuck in wait(NULL).

Fix the issue by killing the child before jumping to skel_open_load_failure.

The bug was discovered while compiling all of selftests with -O1 instead of -O2
which caused progs/test_send_signal_kern.c to fail to load.

Signed-off-by: Alexei Starovoitov <[email protected]>
Tested-by: Yonghong Song <[email protected]>
@kernel-patches-daemon-bpf
Copy link
Author

At least one diff in series https://patchwork.kernel.org/project/netdevbpf/list/?series=1022813 expired. Closing PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant