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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAAVpQUAyPwyG=aSnv-2w7g3dqhB3BLXGoo5VmbSNqQ0txpqqWQ@mail.gmail.com>
Date: Wed, 12 Nov 2025 22:10:32 -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 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.

In that sense, the comment above accept() sounds a
bit weird ;)


>
> 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

Powered by Openwall GNU/*/Linux Powered by OpenVZ