Re: [PATCH v4 4/6] selftest/landlock: Test file_send_sigiotask by sending out-of-bound message

Mickaël Salaün
2024-09-09T10:40:10Z

This test does not cover hook_file_send_sigiotask(): the is_scoped
variable is never set to true.

On Fri, Sep 06, 2024 at 03:30:06PM -0600, Tahera Fahimi wrote:
> This patch adds a test to verify handling the signal scoping mechanism
> in file_send_sigiotask by triggering SIGURG through receiving an
> out-of-bound message in UNIX sockets.
>
> Signed-off-by: Tahera Fahimi <fahimitahera@gmail.com>
> ---
> V4:
> * Using pipe instead of Poll for synchronization.
> ---
> .../selftests/landlock/scoped_signal_test.c | 99 +++++++++++++++++++
> 1 file changed, 99 insertions(+)
>
> diff --git a/tools/testing/selftests/landlock/scoped_signal_test.c b/tools/testing/selftests/landlock/scoped_signal_test.c
> index c71fb83b7147..630f3a515731 100644
> --- a/tools/testing/selftests/landlock/scoped_signal_test.c
> +++ b/tools/testing/selftests/landlock/scoped_signal_test.c
> @@ -269,4 +269,103 @@ TEST(signal_scoping_threads)
> EXPECT_EQ(0, close(thread_pipe[1]));
> }
>
> +#define SOCKET_PATH "/tmp/unix_sock_test"

We must not create file on absolute paths because concurrent executions
or previous ones could interfer with the tests. Why not use an abstract
unix socket created with set_unix_address()?

> +
> +const short backlog = 10;
> +
> +static volatile sig_atomic_t signal_received;
> +
> +static void handle_sigurg(int sig)
> +{
> + if (sig == SIGURG)
> + signal_received = 1;
> + else
> + signal_received = -1;
> +}
> +
> +static int setup_signal_handler(int signal)
> +{
> + struct sigaction sa;
> +
> + sa.sa_handler = handle_sigurg;
> + sigemptyset(&sa.sa_mask);
> + sa.sa_flags = SA_SIGINFO | SA_RESTART;
> + return sigaction(SIGURG, &sa, NULL);
> +}
> +
> +/*
> + * Sending an out of bound message will trigger the SIGURG signal
> + * through file_send_sigiotask.
> + */
> +TEST(test_sigurg_socket)
> +{
> + int sock_fd, recv_sock;
> + struct sockaddr_un addr, paddr;
> + socklen_t size;
> + char oob_buf, buffer;
> + int status;
> + int pipe_parent[2], pipe_child[2];
> + pid_t child;
> +
> + ASSERT_EQ(0, pipe2(pipe_parent, O_CLOEXEC));
> + ASSERT_EQ(0, pipe2(pipe_child, O_CLOEXEC));
> +
> + memset(&addr, 0, sizeof(addr));
> + addr.sun_family = AF_UNIX;
> + snprintf(addr.sun_path, sizeof(addr.sun_path), "%s", SOCKET_PATH);
> + unlink(SOCKET_PATH);
> + size = sizeof(addr);
> +
> + child = fork();
> + ASSERT_LE(0, child);
> + if (child == 0) {
> + oob_buf = '.';
> +
> + ASSERT_EQ(0, close(pipe_parent[1]));
> + ASSERT_EQ(0, close(pipe_child[0]));
> +
> + sock_fd = socket(AF_UNIX, SOCK_STREAM, 0);
> + ASSERT_NE(-1, sock_fd);
> +
> + ASSERT_EQ(1, read(pipe_parent[0], &buffer, 1));
> + ASSERT_EQ(0, connect(sock_fd, &addr, sizeof(addr)));
> +
> + ASSERT_EQ(1, read(pipe_parent[0], &buffer, 1));
> + ASSERT_NE(-1, send(sock_fd, &oob_buf, 1, MSG_OOB));
> + ASSERT_EQ(1, write(pipe_child[1], ".", 1));
> +
> + EXPECT_EQ(0, close(sock_fd));
> +
> + _exit(_metadata->exit_code);
> + return;
> + }
> + ASSERT_EQ(0, close(pipe_parent[0]));
> + ASSERT_EQ(0, close(pipe_child[1]));
> +
> + sock_fd = socket(AF_UNIX, SOCK_STREAM, 0);
> + ASSERT_NE(-1, sock_fd);
> + ASSERT_EQ(0, bind(sock_fd, &addr, size));
> + ASSERT_EQ(0, listen(sock_fd, backlog));
> +
> + ASSERT_NE(-1, setup_signal_handler(SIGURG));
> + ASSERT_EQ(1, write(pipe_parent[1], ".", 1));
> + recv_sock = accept(sock_fd, &paddr, &size);
> + ASSERT_NE(-1, recv_sock);
> +
> + create_scoped_domain(_metadata, LANDLOCK_SCOPED_SIGNAL);
> +
> + ASSERT_NE(-1, fcntl(recv_sock, F_SETOWN, getpid()));
> + ASSERT_EQ(1, write(pipe_parent[1], ".", 1));
> + ASSERT_EQ(1, read(pipe_child[0], &buffer, 1));
> + ASSERT_EQ(1, recv(recv_sock, &oob_buf, 1, MSG_OOB));
> +
> + ASSERT_EQ(1, signal_received);
> + EXPECT_EQ(0, close(sock_fd));
> + EXPECT_EQ(0, close(recv_sock));
> + ASSERT_EQ(child, waitpid(child, &status, 0));
> + if (WIFSIGNALED(status) || !WIFEXITED(status) ||
> + WEXITSTATUS(status) != EXIT_SUCCESS)
> + _metadata->exit_code = KSFT_FAIL;
> +}
> +
> TEST_HARNESS_MAIN
> --
> 2.34.1
>