| 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: <7c4070ed-1702-4288-90c6-7edb90468718@yahoo.com>
Date: Thu, 30 Oct 2025 14:45:05 +0100
From: Sunday Adelodun <adelodunolaoluwa@...oo.com>
To: Kuniyuki Iwashima <kuniyu@...gle.com>
Cc: "=David S . Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>,
Shuah Khan <shuah@...nel.org>, Shuah Khan <skhan@...uxfoundation.org>,
David Hunter <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 v2] selftests: af_unix: Add tests for ECONNRESET and EOF
semantics
On 10/28/25 19:28, Kuniyuki Iwashima wrote:
> On Sat, Oct 25, 2025 at 12:03 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 cover:
>>
>> 1. EOF returned when a SOCK_STREAM peer closes normally.
>> 2. ECONNRESET returned when a SOCK_STREAM peer closes with unread data.
>> 3. SOCK_DGRAM sockets not returning ECONNRESET on peer close.
>>
>> 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 from v1:
>>
>> - Patch prefix updated to selftest: af_unix:.
>>
>> - All mentions of “UNIX” changed to AF_UNIX.
>>
>> - Removed BSD references from comments.
>>
>> - Shared setup refactored using FIXTURE_VARIANT().
>>
>> - Cleanup moved to FIXTURE_TEARDOWN() to always run.
>>
>> - Tests consolidated to reduce duplication: EOF, ECONNRESET, SOCK_DGRAM peer close.
>>
>> - Corrected ASSERT usage and initialization style.
>>
>> - Makefile updated for new directory af_unix.
>>
>> tools/testing/selftests/net/af_unix/Makefile | 1 +
>> .../selftests/net/af_unix/unix_connreset.c | 161 ++++++++++++++++++
>> 2 files changed, 162 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 \
>> # 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..c65ec997d77d
>> --- /dev/null
>> +++ b/tools/testing/selftests/net/af_unix/unix_connreset.c
>> @@ -0,0 +1,161 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Selftest for AF_UNIX socket close and ECONNRESET behaviour.
>> + *
>> + * 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.
>> + *
>> + * 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 */
>> +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",
>> +};
> Let's add coverage for SOCK_SEQPACKET,
> which needs listen() / connect() but other semantics
> are similar to SOCK_DGRAM.
I will add it through:
if (variant->socket_type == SOCK_STREAM ||
variant->socket_type == SOCK_SEQPACKET)
in both the setup and teardown fixtures with a little bit of modification
where necessary (especially in the setup fixture).
And also the fixture_variant_add macro.
>> +
>> +FIXTURE_SETUP(unix_sock)
>> +{
>> + struct sockaddr_un addr = {};
>> + int err;
>> +
>> + addr.sun_family = AF_UNIX;
>> + strcpy(addr.sun_path, SOCK_PATH);
>> +
>> + 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) {
>> + err = listen(self->server, 1);
>> + ASSERT_EQ(0, err);
>> +
>> + self->client = socket(AF_UNIX, SOCK_STREAM, 0);
>> + 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)
>> + 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)
>> + SKIP(return, "This test only applies to SOCK_STREAM");
> Instead of skipping, let's define final ASSERT() results
> for each type.
>
> Same for other 2 tests.
can I use a switch statement in all the tests? say, for example
test1:
...
switch (variant->socket_type) {
case SOCK_STREAM:
case SOCK_SEQPACKET:
ASSERT_EQ(0, n);
case SOCK_DGRAM:
ASSERT(-1, n);
ASSERT_EQ(EAGAIN, errno);
break;
}
...
test2:
...
switch (variant->socket_type) {
case SOCK_STREAM:
case SOCK_SEQPACKET:
ASSERT_EQ(-1, n);
ASSERT_EQ(ECONNRESET, errno);
break;
case SOCK_DGRAM:
ASSERT(-1, n);
ASSERT_EQ(EAGAIN, errno);
break;
}
...
test 3:
...
switch (variant->socket_type) {
case SOCK_STREAM:
case SOCK_SEQPACKET:
ASSERT_EQ(-1, n);
ASSERT_EQ(ECONNRESET, errno);
break;
case SOCK_DGRAM:
ASSERT(-1, n);
ASSERT_EQ(EAGAIN, errno);
break;
}
...
if not these, could you kindly shed more light to what you meant
>
>
>> +
>> + /* Peer closes normally */
>> + 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 (n == -1)
>> + ASSERT_EQ(ECONNRESET, errno);
> ... otherwise, we don't see an error here
>
>> +
>> + if (n != -1)
>> + ASSERT_EQ(0, n);
> and this can be checked unconditionally.
did you mean I should remove the if (n != -1) ASSERT_EQ(0, n); part?
>
>> +}
>> +
>> +/* Test 2: peer closes with unread data */
>> +TEST_F(unix_sock, reset_unread)
>> +{
>> + char buf[16] = {};
>> + ssize_t n;
>> +
>> + if (variant->socket_type != SOCK_STREAM)
>> + SKIP(return, "This test only applies to SOCK_STREAM");
>> +
>> + /* 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));
>> + ASSERT_EQ(-1, n);
>> + ASSERT_EQ(ECONNRESET, errno);
>> +}
>> +
>> +/* Test 3: SOCK_DGRAM peer close */
>> +TEST_F(unix_sock, dgram_reset)
>> +{
>> + char buf[16] = {};
>> + ssize_t n;
>> +
>> + if (variant->socket_type != SOCK_DGRAM)
>> + SKIP(return, "This test only applies to SOCK_DGRAM");
>> +
>> + send(self->client, "hello", 5, 0);
>> + 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));
>> + /* Expect EAGAIN because there is no datagram and peer is closed. */
>> + ASSERT_EQ(-1, n);
>> + ASSERT_EQ(EAGAIN, errno);
>> +}
>> +
>> +TEST_HARNESS_MAIN
>> +
>> --
>> 2.43.0
>>
Thank you very much for the review and comments.
I will send v3 after you review the questions/clarifications I asked in
my previous reply.
Apologies for the delay in responding.
Thanks again for the guidance and patience, really appreciate it.
Powered by blists - more mailing lists