| 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: <CAAVpQUBvLN=sRVb8cbMngwm3o=KZkVOeCYdyi2p5sYjjZQU=HQ@mail.gmail.com>
Date: Thu, 13 Nov 2025 00:43:34 -0800
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, horms@...nel.org, shuah@...nel.org,
skhan@...uxfoundation.org, david.hunter.linux@...il.com,
linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
linux-kselftest@...r.kernel.org,
linux-kernel-mentees@...ts.linuxfoundation.org
Subject: Re: [PATCH v5] selftests: af_unix: Add tests for ECONNRESET and EOF semantics
On Wed, Nov 12, 2025 at 11:19 PM Sunday Adelodun
<adelodunolaoluwa@...oo.com> wrote:
>
> Add selftests to verify and document Linux’s intended behaviour for
> UNIX domain sockets (SOCK_STREAM and SOCK_DGRAM) when a peer closes.
> The tests verify that:
>
> 1. SOCK_STREAM returns EOF when the peer closes normally.
> 2. SOCK_STREAM returns ECONNRESET if the peer closes with unread data.
> 3. SOCK_SEQPACKET returns EOF when the peer closes normally.
> 4. SOCK_SEQPACKET returns ECONNRESET if the peer closes with unread data.
> 5. SOCK_DGRAM does not return ECONNRESET when the peer closes.
>
> This follows up on review feedback suggesting a selftest to clarify
> Linux’s semantics.
>
> Suggested-by: Kuniyuki Iwashima <kuniyu@...gle.com>
> Signed-off-by: Sunday Adelodun <adelodunolaoluwa@...oo.com>
> ---
> Changelog:
>
> changes made in v4 to v5:
> 1. Moved the send() call before the socket type check in Test 2 to ensure
> the unread data behavior is tested for SOCK_DGRAM as well.
>
> 2. Removed the misleading commend about accept() for clarity.
>
> 3. Applied indentation fixes for style consistency
> (alignment with open parenthesis).
>
> 4. Minor comment and formatting cleanups for clarity and adherence
> to kernel coding style.
>
> tools/testing/selftests/net/.gitignore | 1 +
> tools/testing/selftests/net/af_unix/Makefile | 1 +
> .../selftests/net/af_unix/unix_connreset.c | 178 ++++++++++++++++++
> 3 files changed, 180 insertions(+)
> create mode 100644 tools/testing/selftests/net/af_unix/unix_connreset.c
>
> diff --git a/tools/testing/selftests/net/.gitignore b/tools/testing/selftests/net/.gitignore
> index 439101b518ee..e89a60581a13 100644
> --- a/tools/testing/selftests/net/.gitignore
> +++ b/tools/testing/selftests/net/.gitignore
> @@ -65,3 +65,4 @@ udpgso
> udpgso_bench_rx
> udpgso_bench_tx
> unix_connect
> +unix_connreset
> diff --git a/tools/testing/selftests/net/af_unix/Makefile b/tools/testing/selftests/net/af_unix/Makefile
> index de805cbbdf69..5826a8372451 100644
> --- a/tools/testing/selftests/net/af_unix/Makefile
> +++ b/tools/testing/selftests/net/af_unix/Makefile
> @@ -7,6 +7,7 @@ TEST_GEN_PROGS := \
> scm_pidfd \
> scm_rights \
> unix_connect \
> + unix_connreset \
> # end of TEST_GEN_PROGS
>
> include ../../lib.mk
> diff --git a/tools/testing/selftests/net/af_unix/unix_connreset.c b/tools/testing/selftests/net/af_unix/unix_connreset.c
> new file mode 100644
> index 000000000000..9cb0f48597eb
> --- /dev/null
> +++ b/tools/testing/selftests/net/af_unix/unix_connreset.c
> @@ -0,0 +1,178 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Selftest for AF_UNIX socket close and ECONNRESET behaviour.
> + *
> + * This test verifies:
> + * 1. SOCK_STREAM returns EOF when the peer closes normally.
> + * 2. SOCK_STREAM returns ECONNRESET if peer closes with unread data.
> + * 3. SOCK_SEQPACKET returns EOF when the peer closes normally.
> + * 4. SOCK_SEQPACKET returns ECONNRESET if the peer closes with unread data.
> + * 5. SOCK_DGRAM does not return ECONNRESET when the peer closes.
> + *
> + * These tests document the intended Linux behaviour.
> + *
> + */
> +
> +#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/af_unix_connreset.sock"
> +
> +static void remove_socket_file(void)
> +{
> + unlink(SOCK_PATH);
> +}
> +
> +FIXTURE(unix_sock)
> +{
> + int server;
> + int client;
> + int child;
> +};
> +
> +FIXTURE_VARIANT(unix_sock)
> +{
> + int socket_type;
> + const char *name;
> +};
> +
> +FIXTURE_VARIANT_ADD(unix_sock, stream) {
> + .socket_type = SOCK_STREAM,
> + .name = "SOCK_STREAM",
> +};
> +
> +FIXTURE_VARIANT_ADD(unix_sock, dgram) {
> + .socket_type = SOCK_DGRAM,
> + .name = "SOCK_DGRAM",
> +};
> +
> +FIXTURE_VARIANT_ADD(unix_sock, seqpacket) {
> + .socket_type = SOCK_SEQPACKET,
> + .name = "SOCK_SEQPACKET",
> +};
> +
> +FIXTURE_SETUP(unix_sock)
> +{
> + struct sockaddr_un addr = {};
> + int err;
> +
> + addr.sun_family = AF_UNIX;
> + strcpy(addr.sun_path, SOCK_PATH);
> + remove_socket_file();
> +
> + self->server = socket(AF_UNIX, variant->socket_type, 0);
> + ASSERT_LT(-1, self->server);
> +
> + err = bind(self->server, (struct sockaddr *)&addr, sizeof(addr));
> + ASSERT_EQ(0, err);
> +
> + if (variant->socket_type == SOCK_STREAM ||
> + variant->socket_type == SOCK_SEQPACKET) {
> + err = listen(self->server, 1);
> + ASSERT_EQ(0, err);
> + }
> +
> + self->client = socket(AF_UNIX, variant->socket_type | SOCK_NONBLOCK, 0);
> + ASSERT_LT(-1, self->client);
> +
> + err = connect(self->client, (struct sockaddr *)&addr, sizeof(addr));
> + ASSERT_EQ(0, err);
> +}
> +
> +FIXTURE_TEARDOWN(unix_sock)
> +{
> + if ((variant->socket_type == SOCK_STREAM ||
> + variant->socket_type == SOCK_SEQPACKET) & self->child > 0)
Sorry for missing this one, but NIPA caught this.
see: https://netdev.bots.linux.dev/static/nipa/1022816/14311938/build_tools/stderr
+unix_connreset.c: In function ‘unix_sock_teardown’:
+unix_connreset.c:92:68: warning: suggest parentheses around
comparison in operand of ‘&’ [-Wparentheses]
+ 92 | variant->socket_type == SOCK_SEQPACKET) & self->child > 0)
+ | ~~~~~~~~~~~~^~~
I think you can simply remove the "& self->child >0" part
because you don't check that for self->server below anyway.
Thanks
> + close(self->child);
> +
> + close(self->client);
> + close(self->server);
> + remove_socket_file();
> +}
> +
> +/* Test 1: peer closes normally */
> +TEST_F(unix_sock, eof)
> +{
> + char buf[16] = {};
> + ssize_t n;
> +
> + if (variant->socket_type == SOCK_STREAM ||
> + variant->socket_type == SOCK_SEQPACKET) {
> + self->child = accept(self->server, NULL, NULL);
> + ASSERT_LT(-1, self->child);
> +
> + close(self->child);
> + } else {
> + close(self->server);
> + }
> +
> + n = recv(self->client, buf, sizeof(buf), 0);
> +
> + if (variant->socket_type == SOCK_STREAM ||
> + variant->socket_type == SOCK_SEQPACKET) {
> + ASSERT_EQ(0, n);
> + } else {
> + ASSERT_EQ(-1, n);
> + ASSERT_EQ(EAGAIN, errno);
> + }
> +}
> +
> +/* Test 2: peer closes with unread data */
> +TEST_F(unix_sock, reset_unread_behavior)
> +{
> + char buf[16] = {};
> + ssize_t n;
> +
> + /* Send data that will remain unread */
> + send(self->client, "hello", 5, 0);
> +
> + if (variant->socket_type == SOCK_DGRAM) {
> + /* No real connection, just close the server */
> + close(self->server);
> + } else {
> + /* Accept client connection */
> + self->child = accept(self->server, NULL, NULL);
> + ASSERT_LT(-1, self->child);
> +
> + /* Peer closes before client reads */
> + close(self->child);
> + }
> +
> + n = recv(self->client, buf, sizeof(buf), 0);
> + ASSERT_EQ(-1, n);
> +
> + if (variant->socket_type == SOCK_STREAM ||
> + variant->socket_type == SOCK_SEQPACKET) {
> + ASSERT_EQ(ECONNRESET, errno);
> + } else {
> + ASSERT_EQ(EAGAIN, errno);
> + }
> +}
> +
> +/* Test 3: closing unaccepted (embryo) server socket should reset client. */
> +TEST_F(unix_sock, reset_closed_embryo)
> +{
> + char buf[16] = {};
> + ssize_t n;
> +
> + if (variant->socket_type == SOCK_DGRAM)
> + SKIP(return, "This test only applies to SOCK_STREAM and SOCK_SEQPACKET");
> +
> + /* Close server without accept()ing */
> + close(self->server);
> +
> + n = recv(self->client, buf, sizeof(buf), 0);
> +
> + ASSERT_EQ(-1, n);
> + ASSERT_EQ(ECONNRESET, errno);
> +}
> +
> +TEST_HARNESS_MAIN
> +
> --
> 2.43.0
>
Powered by blists - more mailing lists