| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAAVpQUAX-+5cOCaZrA1DbMTLrUEhCsK=6JSHAQgSNhbOyQ06MA@mail.gmail.com>
Date: Thu, 23 Oct 2025 20:00:28 -0700
From: Kuniyuki Iwashima <kuniyu@...gle.com>
To: Sunday Adelodun <adelodunolaoluwa@...oo.com>
Cc: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
pabeni@...hat.com, shuah@...nel.org, horms@...nel.org,
linux-kselftest@...r.kernel.org, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org, skhan@...uxfoundation.org,
david.hunter.linux@...il.com
Subject: Re: [PATCH] selftests/unix: Add test for ECONNRESET and EOF behaviour
Thanks for adding tests.
> [PATCH] selftests/unix: Add test for ECONNRESET and EOF behaviour
nit: The common prefix is "selftest: af_unix:".
On Thu, Oct 23, 2025 at 9:59 AM Sunday Adelodun
<adelodunolaoluwa@...oo.com> wrote:
>
> Add selftests verifying the EOF and ECONNRESET behaviour of
> UNIX domain sockets (SOCK_STREAM and SOCK_DGRAM). The tests document
> Linux's semantics and clarify the long-standing differences with BSD.
>
> Suggested-by: Kuniyuki Iwashima <kuniyu@...gle.com>
> Signed-off-by: Sunday Adelodun <adelodunolaoluwa@...oo.com>
> ---
> tools/testing/selftests/net/unix/Makefile | 5 +
> .../selftests/net/unix/test_unix_connreset.c | 147 ++++++++++++++++++
> 2 files changed, 152 insertions(+)
> create mode 100644 tools/testing/selftests/net/unix/Makefile
> create mode 100644 tools/testing/selftests/net/unix/test_unix_connreset.c
The test for af_unix is placed under tools/testing/selftests/net/af_unix.
>
> diff --git a/tools/testing/selftests/net/unix/Makefile b/tools/testing/selftests/net/unix/Makefile
> new file mode 100644
> index 000000000000..a52992ba23d9
> --- /dev/null
> +++ b/tools/testing/selftests/net/unix/Makefile
> @@ -0,0 +1,5 @@
> +# SPDX-License-Identifier: GPL-2.0
> +TEST_GEN_PROGS := test_unix_connreset
> +
> +include ../../lib.mk
> +
> diff --git a/tools/testing/selftests/net/unix/test_unix_connreset.c b/tools/testing/selftests/net/unix/test_unix_connreset.c
> new file mode 100644
> index 000000000000..a8720c7565cb
> --- /dev/null
> +++ b/tools/testing/selftests/net/unix/test_unix_connreset.c
> @@ -0,0 +1,147 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Selftest for UNIX socket close and ECONNRESET behaviour.
nit: s/UNIX/AF_UNIX/
> + *
> + * This test verifies that:
> + * 1. SOCK_STREAM sockets return EOF when peer closes normally.
> + * 2. SOCK_STREAM sockets return ECONNRESET if peer closes with unread data.
> + * 3. SOCK_DGRAM sockets do not return ECONNRESET when peer closes,
> + * unlike BSD where this error is observed.
> + *
> + * These tests document the intended Linux behaviour, distinguishing it from BSD.
I'd not mention BSD as it could be outdated again.
> + *
> + */
> +
> +#define _GNU_SOURCE
> +#include <stdlib.h>
> +#include <string.h>
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <errno.h>
> +#include <sys/socket.h>
> +#include <sys/un.h>
> +#include "../../kselftest_harness.h"
> +
> +#define SOCK_PATH "/tmp/test_unix_connreset.sock"
> +
> +static void remove_socket_file(void)
> +{
> + unlink(SOCK_PATH);
> +}
> +
> +/* Test 1: peer closes normally */
> +TEST(stream_eof)
I think most of the code can be shared by defining
FIXTURE_VARIANT().
e.g. variant->unread_data can consolidate Test 1&2.
> +{
> + int server, client, child;
> + struct sockaddr_un addr = {0};
> + char buf[16] = {0};
IIRC, {0} only initialises the first entry and we had a problem
in kernel code, so simply use "= {};" everywhere.
> + ssize_t n;
> +
> + server = socket(AF_UNIX, SOCK_STREAM, 0);
Try using variant->type for SOCK_STREAM,
SOCK_DGRAM, and SOCK_SEQPACKET.
See unix_connect.c, or you could reuse the fixtures
of err == 0 there.
> + ASSERT_GE(server, 0);
nit: The 1st arg is "expected", and the 2nd is "seen",
so ASSERT_NE(-1, server) (or ASSERT_LT(-1, server)).
Same for other places.
> +
> + addr.sun_family = AF_UNIX;
> + strcpy(addr.sun_path, SOCK_PATH);
> + remove_socket_file();
> +
> + ASSERT_EQ(bind(server, (struct sockaddr *)&addr, sizeof(addr)), 0);
I personally feel easy to read this style:
err = bind();
ASSERT_EQ(0, err);
> + ASSERT_EQ(listen(server, 1), 0);
> +
> + client = socket(AF_UNIX, SOCK_STREAM, 0);
> + ASSERT_GE(client, 0);
> + ASSERT_EQ(connect(client, (struct sockaddr *)&addr, sizeof(addr)), 0);
> +
> + child = accept(server, NULL, NULL);
> + ASSERT_GE(child, 0);
> +
> + /* Peer closes normally */
> + close(child);
> +
> + n = recv(client, buf, sizeof(buf), 0);
> + EXPECT_EQ(n, 0);
> + TH_LOG("recv=%zd errno=%d (%s)", n, errno, strerror(errno));
I printed errno just for visibility, and you can simply use
ASSERT here too like
if (n == -1)
ASSERT_EQ(ECONNRESET, errno)
(I'm assuming Test 1 & 2 will share most code)
> +
> + close(client);
> + close(server);
> + remove_socket_file();
This will not be executed if the program fails at ASSERT_XX(),
so move it to FIXTURE_TEARDOWN().
> +}
> +
> +/* Test 2: peer closes with unread data */
> +TEST(stream_reset_unread)
> +{
> + int server, client, child;
> + struct sockaddr_un addr = {0};
> + char buf[16] = {0};
> + ssize_t n;
> +
> + server = socket(AF_UNIX, SOCK_STREAM, 0);
> + ASSERT_GE(server, 0);
> +
> + addr.sun_family = AF_UNIX;
> + strcpy(addr.sun_path, SOCK_PATH);
> + remove_socket_file();
> +
> + ASSERT_EQ(bind(server, (struct sockaddr *)&addr, sizeof(addr)), 0);
> + ASSERT_EQ(listen(server, 1), 0);
> +
> + client = socket(AF_UNIX, SOCK_STREAM, 0);
> + ASSERT_GE(client, 0);
> + ASSERT_EQ(connect(client, (struct sockaddr *)&addr, sizeof(addr)), 0);
> +
> + child = accept(server, NULL, NULL);
> + ASSERT_GE(child, 0);
> +
> + /* Send data that will remain unread by client */
> + send(client, "hello", 5, 0);
> + close(child);
> +
> + n = recv(client, buf, sizeof(buf), 0);
> + EXPECT_LT(n, 0);
> + EXPECT_EQ(errno, ECONNRESET);
> + TH_LOG("recv=%zd errno=%d (%s)", n, errno, strerror(errno));
> +
> + close(client);
> + close(server);
> + remove_socket_file();
> +}
> +
> +/* Test 3: SOCK_DGRAM peer close */
> +TEST(dgram_reset)
> +{
> + int server, client;
> + int flags;
> + struct sockaddr_un addr = {0};
> + char buf[16] = {0};
> + ssize_t n;
> +
> + server = socket(AF_UNIX, SOCK_DGRAM, 0);
> + ASSERT_GE(server, 0);
> +
> + addr.sun_family = AF_UNIX;
> + strcpy(addr.sun_path, SOCK_PATH);
> + remove_socket_file();
> +
> + ASSERT_EQ(bind(server, (struct sockaddr *)&addr, sizeof(addr)), 0);
> +
> + client = socket(AF_UNIX, SOCK_DGRAM, 0);
> + ASSERT_GE(client, 0);
> + ASSERT_EQ(connect(client, (struct sockaddr *)&addr, sizeof(addr)), 0);
> +
> + send(client, "hello", 5, 0);
> + close(server);
> +
> + flags = fcntl(client, F_GETFL, 0);
> + fcntl(client, F_SETFL, flags | O_NONBLOCK);
You can save fcntl() with socket(..., ... | SOCK_NONBLOCK, ...).
> +
> + n = recv(client, buf, sizeof(buf), 0);
> + TH_LOG("recv=%zd errno=%d (%s)", n, errno, strerror(errno));
> + /* Expect EAGAIN or EWOULDBLOCK because there is no datagram and peer is closed. */
> + EXPECT_LT(n, 0);
> + EXPECT_TRUE(errno == EAGAIN);
> +
> + close(client);
> + remove_socket_file();
> +}
> +
> +TEST_HARNESS_MAIN
> +
> --
> 2.43.0
>
Powered by blists - more mailing lists