| 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: <233cb349-b7ea-42cf-8cc4-ce08cf3179ee@yahoo.com>
Date: Thu, 13 Nov 2025 08:29:10 +0100
From: Sunday Adelodun <adelodunolaoluwa@...oo.com>
To: Kuniyuki Iwashima <kuniyu@...gle.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 11/13/25 07:52, Kuniyuki Iwashima wrote:
> 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!
Sent.
I removed the comment, but unfortunately I didn't know it didn't save.
so, the comment was sent along with patch.
Thank you for the guidance and patience.
I am open to any feedback if any.
>
>> 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