| 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: <CAAVpQUDX2LcWu+u5CHSJUoPnHLuGNoPzEZuV+K67WoPUUvymdg@mail.gmail.com>
Date: Wed, 12 Nov 2025 22:52:54 -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 v4] selftests: af_unix: Add tests for ECONNRESET and EOF semantics
On Wed, Nov 12, 2025 at 10:41 PM Sunday Adelodun
<adelodunolaoluwa@...oo.com> wrote:
>
> On 11/13/25 07:10, Kuniyuki Iwashima wrote:
> > On Wed, Nov 12, 2025 at 9:25 PM Sunday Adelodun
> > <adelodunolaoluwa@...oo.com> wrote:
> >> On 11/12/25 23:04, Kuniyuki Iwashima wrote:
> >>> On Wed, Nov 12, 2025 at 1:20 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>
> >>>> ---
> >>>> 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..9413f8a0814f
> >>>> --- /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)
> >>>> + 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;
> >>>> +
> >>>> + if (variant->socket_type == SOCK_DGRAM) {
> >>>> + /* No real connection, just close the server */
> >>>> + close(self->server);
> >>>> + } else {
> >>>> + /* Establish full connection first */
> >>>> + self->child = accept(self->server, NULL, NULL);
> >>>> + ASSERT_LT(-1, self->child);
> >>>> +
> >>>> + /* Send data that will remain unread */
> >>>> + send(self->client, "hello", 5, 0);
> >>> Could you move this send() before "if (...)" because we want
> >>> to test unread_data behaviour for SOCK_DGRAM too ?
> >>>
> >>> Otherwise looks good, so with that fixed:
> >>>
> >>> Reviewed-by: Kuniyuki Iwashima <kuniyu@...gle.com>
> >>>
> >>> Thanks!
> >> Thank you for the prompt response.
> >> I thought of putting the send before the if the statement , but I was afraid
> >> STREAM and SEQPACKET connections won't be accepted before data sending.
> > connect() create the paired child socket and accept()
> > allocates a file descriptor to expose the socket to user
> > space.
> Thank you for this explanation.
> >
> > In that sense, the comment above accept() sounds a
> > bit weird ;)
> I understand.
> I am changing it to
> /* Accept client connection */
I'd remove it since it just repeats the code, but up to you.
Thanks!
>
> If this is accepted, I will send v5 immediately
>
> Thank you for your guidance.
> >
> >
> >> I will start working on v5.
> >>
> >> That part will look like this now:
> >> /* 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 {
> >> /* Establish full connection first */
> >> 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);
> >> }
> >> }
> >>
> >> Thank you once again.
> >>>
> >>>> +
> >>>> + /* 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