Thread (7 messages) 7 messages, 2 authors, 2025-11-12

Re: [PATCH v3] selftests: af_unix: Add tests for ECONNRESET and EOF semantics

From: Sunday Adelodun <hidden>
Date: 2025-11-04 11:03:07
Also in: linux-kernel-mentees, linux-kselftest, lkml

On 11/4/25 01:30, Kuniyuki Iwashima wrote:
On Mon, Nov 3, 2025 at 4:08 PM Sunday Adelodun
[off-list ref] wrote:
quoted
On 11/2/25 08:32, Kuniyuki Iwashima wrote:
quoted
On Sat, Nov 1, 2025 at 10:23 AM Sunday Adelodun
[off-list ref] wrote:
quoted
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@google.com>
Signed-off-by: Sunday Adelodun <redacted>
---
   tools/testing/selftests/net/af_unix/Makefile  |   1 +
   .../selftests/net/af_unix/unix_connreset.c    | 179 ++++++++++++++++++
   2 files changed, 180 insertions(+)
   create mode 100644 tools/testing/selftests/net/af_unix/unix_connreset.c
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 \
patchwork caught this test is not added to .gitignore.
https://patchwork.kernel.org/project/netdevbpf/patch/20251101172230.10179-1-adelodunolaoluwa@yahoo.com/

Could you add it to this file ?

tools/testing/selftests/net/.gitignore
Oh, thank you for this. will add it
quoted
quoted
   # 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..6f43435d96e2
--- /dev/null
+++ b/tools/testing/selftests/net/af_unix/unix_connreset.c
@@ -0,0 +1,179 @@
+// 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;
+};
+
+/* Define variants: stream and datagram */
nit: outdated, maybe simply remove ?
oh..skipped me.
will do so.
quoted
quoted
+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) {
patchwork caught mis-alignment here and other places.

I'm using this for emacs, and other editors will have a similar config.

(setq-default c-default-style "linux")

You can check if lines are aligned properly by

$ git show --format=email | ./scripts/checkpatch.pl

quoted
+               err = listen(self->server, 1);
+               ASSERT_EQ(0, err);
+
+               self->client = socket(AF_UNIX, variant->socket_type, 0);
Could you add SOCK_NONBLOCK here too ?
This is noted
quoted
quoted
+               ASSERT_LT(-1, self->client);
+
+               err = connect(self->client, (struct sockaddr *)&addr, sizeof(addr));
+               ASSERT_EQ(0, err);
+
+               self->child = accept(self->server, NULL, NULL);
+               ASSERT_LT(-1, self->child);
+       } else {
+               /* Datagram: bind and connect only */
+               self->client = socket(AF_UNIX, SOCK_DGRAM | 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)
+               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;
+
+       /* Peer closes normally */
+       if (variant->socket_type == SOCK_STREAM ||
+               variant->socket_type == SOCK_SEQPACKET)
+               close(self->child);
+       else
+               close(self->server);
+
+       n = recv(self->client, buf, sizeof(buf), 0);
+       TH_LOG("%s: recv=%zd errno=%d (%s)", variant->name, n, errno, strerror(errno));
errno is undefined if not set, and same for strerror(errno).

Also, if ASSERT_XXX() below fails, the same information
(test case, errno) is logged.  So, TH_LOG() seems unnecessary.

Maybe try modifying the condition below and see how the
assertion is logged.
Oh..thank you. Didn't it through that way.
I understand.
I will remove the TH_LOG()'s
quoted
quoted
+       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)
+{
+       char buf[16] = {};
+       ssize_t n;
+
+       /* Send data that will remain unread by client */
+       send(self->client, "hello", 5, 0);
+       close(self->child);
+
+       n = recv(self->client, buf, sizeof(buf), 0);
+       TH_LOG("%s: recv=%zd errno=%d (%s)", variant->name, n, errno, strerror(errno));
+       if (variant->socket_type == SOCK_STREAM ||
+               variant->socket_type == SOCK_SEQPACKET) {
+               ASSERT_EQ(-1, n);
+               ASSERT_EQ(ECONNRESET, errno);
+       } else {
+               ASSERT_EQ(-1, n);
+               ASSERT_EQ(EAGAIN, errno);
+       }
+}
+
+/* Test 3: SOCK_DGRAM peer close */
Now Test 2 and Test 3 look identical ;)
seems so, but the only difference is:

close(self->child); is used in Test 2, while
close(self->server); is used in Test 3.
Maybe I should find a way to collapse Tests 2 and 3 (if statement might
work)

I am just afraid the tests to run will reduce to 6 from 9 and we will have 6
cases passed as against 7 as before.

What do you think?
The name of Test 2 is a bit confusing, which is not true
for SOCK_DGRAM.  So, I'd use "if" to change which fd
to close() depending on the socket type.

Also, close(self->server) does nothing for SOCK_STREAM
and SOCK_SEQPACKET after accept().  Rather, that close()
should have the same effect if self->child is not accept()ed.
(In this case, Skip() for SOCK_DGRAM makes sense)

I think covering that scenario would be nicer.

If interested, you can check the test coverage with this patch.
https://lore.kernel.org/linux-kselftest/20251028024339.2028774-1-kuniyu@google.com/ (local)

Thanks!
Thank you!

kindly check these if any conforms to what it should be:

TEST_F(unix_sock, reset_unread_behavior)
{
         char buf[16] = {};
         ssize_t n;

         /* Send data that will remain unread by client */
         send(self->client, "hello", 5, 0);

         if (variant->socket_type == SOCK_DGRAM) {
                 close(self->server);
         }
         else {
                 if (!self->child)
                         SKIP(return);

                 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)
                 do { ASSERT_EQ(ECONNRESET, errno); } while (0);
         else
                 ASSERT_EQ(EAGAIN, errno);
}

OR

TEST_F(unix_sock, reset_unread_behavior)
{
         char buf[16] = {};
         ssize_t n;

         /* Send data that will remain unread by client */
         send(self->client, "hello", 5, 0);

         if (variant->socket_type == SOCK_DGRAM) {
                 close(self->server);
         }
         else {
                 if (self->child)
                     close(self->child);
                 else
                     close(self->server);
         }

         n = recv(self->client, buf, sizeof(buf), 0);

         ASSERT_EQ(-1, n);

         if (variant->socket_type == SOCK_STREAM ||
                 variant->socket_type == SOCK_SEQPACKET)
                 do { ASSERT_EQ(ECONNRESET, errno); } while (0);
         else
                 ASSERT_EQ(EAGAIN, errno);
}

OR

is there a better way to handle this?

I ran the KCOV_OUTPUT command using the first *TEST_F above* as the Test 
2 and got the output below:
*$ KCOV_OUTPUT=kcov KCOV_SLOTS=8192 
./tools/testing/selftests/net/af_unix/unix_connreset*
TAP version 13
1..6
# Starting 6 tests from 3 test cases.
#  RUN           unix_sock.stream.eof ...
#            OK  unix_sock.stream.eof
ok 1 unix_sock.stream.eof
#  RUN           unix_sock.stream.reset_unread_behavior ...
#            OK  unix_sock.stream.reset_unread_behavior
ok 2 unix_sock.stream.reset_unread_behavior
#  RUN           unix_sock.dgram.eof ...
#            OK  unix_sock.dgram.eof
ok 3 unix_sock.dgram.eof
#  RUN           unix_sock.dgram.reset_unread_behavior ...
#            OK  unix_sock.dgram.reset_unread_behavior
ok 4 unix_sock.dgram.reset_unread_behavior
#  RUN           unix_sock.seqpacket.eof ...
#            OK  unix_sock.seqpacket.eof
ok 5 unix_sock.seqpacket.eof
#  RUN           unix_sock.seqpacket.reset_unread_behavior ...
#            OK  unix_sock.seqpacket.reset_unread_behavior
ok 6 unix_sock.seqpacket.reset_unread_behavior
# PASSED: 6 / 6 tests passed.
# Totals: pass:6 fail:0 xfail:0 xpass:0 skip:0 error:0

Thank you once again for your continuous guidance and patience.
It's a worthy and rewarding learning period for me.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help