Hello,
this small patchset adds POSIX compliant behaviour on writes to the
socket which was shutdowned with 'shutdown()' (both sides - local with
SHUT_WR flag, peer - with SHUT_RD flag). According POSIX we must send
SIGPIPE in such cases (but SIGPIPE is not send when MSG_NOSIGNAL is set).
First patch is implemented in the same way as net/ipv4/tcp.c:tcp_sendmsg_locked().
It uses 'sk_stream_error()' function which handles EPIPE error. Another
way is to use code from net/unix/af_unix.c:unix_stream_sendmsg() where
same logic from 'sk_stream_error()' is implemented "from scratch", but
it doesn't check 'sk_err' field. I think error from this field has more
priority to be returned from syscall. So I guess it is better to reuse
currently implemented 'sk_stream_error()' function.
Test is also added.
Head for this patchset is:
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=9d0cd5d25f7d45bce01bbb3193b54ac24b3a60f3
Arseniy Krasnov (2):
vsock: send SIGPIPE on write to shutdowned socket
test/vsock: shutdowned socket test
net/vmw_vsock/af_vsock.c | 3 +
tools/testing/vsock/vsock_test.c | 138 +++++++++++++++++++++++++++++++
2 files changed, 141 insertions(+)
--
2.25.1
This adds two tests for 'shutdown()' call. It checks that SIGPIPE is
sent when MSG_NOSIGNAL is not set and vice versa. Both flags SHUT_WR
and SHUT_RD are tested.
Signed-off-by: Arseniy Krasnov <[email protected]>
---
tools/testing/vsock/vsock_test.c | 138 +++++++++++++++++++++++++++++++
1 file changed, 138 insertions(+)
diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
index 90718c2fd4ea..21d40a8d881c 100644
--- a/tools/testing/vsock/vsock_test.c
+++ b/tools/testing/vsock/vsock_test.c
@@ -19,6 +19,7 @@
#include <time.h>
#include <sys/mman.h>
#include <poll.h>
+#include <signal.h>
#include "timeout.h"
#include "control.h"
@@ -1170,6 +1171,133 @@ static void test_seqpacket_msg_peek_server(const struct test_opts *opts)
return test_msg_peek_server(opts, true);
}
+static bool have_sigpipe;
+
+static void sigpipe(int signo)
+{
+ have_sigpipe = true;
+}
+
+static void test_stream_check_sigpipe(int fd)
+{
+ ssize_t res;
+
+ have_sigpipe = false;
+
+ res = send(fd, "A", 1, 0);
+ if (res != -1) {
+ fprintf(stderr, "expected send(2) failure, got %zi\n", res);
+ exit(EXIT_FAILURE);
+ }
+
+ if (!have_sigpipe) {
+ fprintf(stderr, "SIGPIPE expected\n");
+ exit(EXIT_FAILURE);
+ }
+
+ have_sigpipe = false;
+
+ res = send(fd, "A", 1, MSG_NOSIGNAL);
+ if (res != -1) {
+ fprintf(stderr, "expected send(2) failure, got %zi\n", res);
+ exit(EXIT_FAILURE);
+ }
+
+ if (have_sigpipe) {
+ fprintf(stderr, "SIGPIPE not expected\n");
+ exit(EXIT_FAILURE);
+ }
+}
+
+static void test_stream_shutwr_client(const struct test_opts *opts)
+{
+ int fd;
+
+ struct sigaction act = {
+ .sa_handler = sigpipe,
+ };
+
+ sigaction(SIGPIPE, &act, NULL);
+
+ fd = vsock_stream_connect(opts->peer_cid, 1234);
+ if (fd < 0) {
+ perror("connect");
+ exit(EXIT_FAILURE);
+ }
+
+ if (shutdown(fd, SHUT_WR)) {
+ perror("shutdown");
+ exit(EXIT_FAILURE);
+ }
+
+ test_stream_check_sigpipe(fd);
+
+ control_writeln("CLIENTDONE");
+
+ close(fd);
+}
+
+static void test_stream_shutwr_server(const struct test_opts *opts)
+{
+ int fd;
+
+ fd = vsock_stream_accept(VMADDR_CID_ANY, 1234, NULL);
+ if (fd < 0) {
+ perror("accept");
+ exit(EXIT_FAILURE);
+ }
+
+ control_expectln("CLIENTDONE");
+
+ close(fd);
+}
+
+static void test_stream_shutrd_client(const struct test_opts *opts)
+{
+ int fd;
+
+ struct sigaction act = {
+ .sa_handler = sigpipe,
+ };
+
+ sigaction(SIGPIPE, &act, NULL);
+
+ fd = vsock_stream_connect(opts->peer_cid, 1234);
+ if (fd < 0) {
+ perror("connect");
+ exit(EXIT_FAILURE);
+ }
+
+ control_expectln("SHUTRDDONE");
+
+ test_stream_check_sigpipe(fd);
+
+ control_writeln("CLIENTDONE");
+
+ close(fd);
+}
+
+static void test_stream_shutrd_server(const struct test_opts *opts)
+{
+ int fd;
+
+ fd = vsock_stream_accept(VMADDR_CID_ANY, 1234, NULL);
+ if (fd < 0) {
+ perror("accept");
+ exit(EXIT_FAILURE);
+ }
+
+ if (shutdown(fd, SHUT_RD)) {
+ perror("shutdown");
+ exit(EXIT_FAILURE);
+ }
+
+ control_writeln("SHUTRDDONE");
+ control_expectln("CLIENTDONE");
+
+ close(fd);
+}
+
static struct test_case test_cases[] = {
{
.name = "SOCK_STREAM connection reset",
@@ -1250,6 +1378,16 @@ static struct test_case test_cases[] = {
.run_client = test_seqpacket_msg_peek_client,
.run_server = test_seqpacket_msg_peek_server,
},
+ {
+ .name = "SOCK_STREAM SHUT_WR",
+ .run_client = test_stream_shutwr_client,
+ .run_server = test_stream_shutwr_server,
+ },
+ {
+ .name = "SOCK_STREAM SHUT_RD",
+ .run_client = test_stream_shutrd_client,
+ .run_server = test_stream_shutrd_server,
+ },
{},
};
--
2.25.1
On Tue, Aug 01, 2023 at 05:17:27PM +0300, Arseniy Krasnov wrote:
>This adds two tests for 'shutdown()' call. It checks that SIGPIPE is
>sent when MSG_NOSIGNAL is not set and vice versa. Both flags SHUT_WR
>and SHUT_RD are tested.
>
>Signed-off-by: Arseniy Krasnov <[email protected]>
>---
> tools/testing/vsock/vsock_test.c | 138 +++++++++++++++++++++++++++++++
> 1 file changed, 138 insertions(+)
>
>diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
>index 90718c2fd4ea..21d40a8d881c 100644
>--- a/tools/testing/vsock/vsock_test.c
>+++ b/tools/testing/vsock/vsock_test.c
>@@ -19,6 +19,7 @@
> #include <time.h>
> #include <sys/mman.h>
> #include <poll.h>
>+#include <signal.h>
>
> #include "timeout.h"
> #include "control.h"
>@@ -1170,6 +1171,133 @@ static void test_seqpacket_msg_peek_server(const struct test_opts *opts)
> return test_msg_peek_server(opts, true);
> }
>
>+static bool have_sigpipe;
^
We should define it as `volatile sig_atomic_t`:
the behavior is undefined if the signal handler refers to any object
[CX] [Option Start] other than errno [Option End] with static storage
duration other than by assigning a value to an object declared as
volatile sig_atomic_t
https://pubs.opengroup.org/onlinepubs/9699919799/functions/signal.html
The rest LGTM!
Thanks,
Stefano
>+
>+static void sigpipe(int signo)
>+{
>+ have_sigpipe = true;
>+}
>+
>+static void test_stream_check_sigpipe(int fd)
>+{
>+ ssize_t res;
>+
>+ have_sigpipe = false;
>+
>+ res = send(fd, "A", 1, 0);
>+ if (res != -1) {
>+ fprintf(stderr, "expected send(2) failure, got %zi\n", res);
>+ exit(EXIT_FAILURE);
>+ }
>+
>+ if (!have_sigpipe) {
>+ fprintf(stderr, "SIGPIPE expected\n");
>+ exit(EXIT_FAILURE);
>+ }
>+
>+ have_sigpipe = false;
>+
>+ res = send(fd, "A", 1, MSG_NOSIGNAL);
>+ if (res != -1) {
>+ fprintf(stderr, "expected send(2) failure, got %zi\n", res);
>+ exit(EXIT_FAILURE);
>+ }
>+
>+ if (have_sigpipe) {
>+ fprintf(stderr, "SIGPIPE not expected\n");
>+ exit(EXIT_FAILURE);
>+ }
>+}
>+
>+static void test_stream_shutwr_client(const struct test_opts *opts)
>+{
>+ int fd;
>+
>+ struct sigaction act = {
>+ .sa_handler = sigpipe,
>+ };
>+
>+ sigaction(SIGPIPE, &act, NULL);
>+
>+ fd = vsock_stream_connect(opts->peer_cid, 1234);
>+ if (fd < 0) {
>+ perror("connect");
>+ exit(EXIT_FAILURE);
>+ }
>+
>+ if (shutdown(fd, SHUT_WR)) {
>+ perror("shutdown");
>+ exit(EXIT_FAILURE);
>+ }
>+
>+ test_stream_check_sigpipe(fd);
>+
>+ control_writeln("CLIENTDONE");
>+
>+ close(fd);
>+}
>+
>+static void test_stream_shutwr_server(const struct test_opts *opts)
>+{
>+ int fd;
>+
>+ fd = vsock_stream_accept(VMADDR_CID_ANY, 1234, NULL);
>+ if (fd < 0) {
>+ perror("accept");
>+ exit(EXIT_FAILURE);
>+ }
>+
>+ control_expectln("CLIENTDONE");
>+
>+ close(fd);
>+}
>+
>+static void test_stream_shutrd_client(const struct test_opts *opts)
>+{
>+ int fd;
>+
>+ struct sigaction act = {
>+ .sa_handler = sigpipe,
>+ };
>+
>+ sigaction(SIGPIPE, &act, NULL);
>+
>+ fd = vsock_stream_connect(opts->peer_cid, 1234);
>+ if (fd < 0) {
>+ perror("connect");
>+ exit(EXIT_FAILURE);
>+ }
>+
>+ control_expectln("SHUTRDDONE");
>+
>+ test_stream_check_sigpipe(fd);
>+
>+ control_writeln("CLIENTDONE");
>+
>+ close(fd);
>+}
>+
>+static void test_stream_shutrd_server(const struct test_opts *opts)
>+{
>+ int fd;
>+
>+ fd = vsock_stream_accept(VMADDR_CID_ANY, 1234, NULL);
>+ if (fd < 0) {
>+ perror("accept");
>+ exit(EXIT_FAILURE);
>+ }
>+
>+ if (shutdown(fd, SHUT_RD)) {
>+ perror("shutdown");
>+ exit(EXIT_FAILURE);
>+ }
>+
>+ control_writeln("SHUTRDDONE");
>+ control_expectln("CLIENTDONE");
>+
>+ close(fd);
>+}
>+
> static struct test_case test_cases[] = {
> {
> .name = "SOCK_STREAM connection reset",
>@@ -1250,6 +1378,16 @@ static struct test_case test_cases[] = {
> .run_client = test_seqpacket_msg_peek_client,
> .run_server = test_seqpacket_msg_peek_server,
> },
>+ {
>+ .name = "SOCK_STREAM SHUT_WR",
>+ .run_client = test_stream_shutwr_client,
>+ .run_server = test_stream_shutwr_server,
>+ },
>+ {
>+ .name = "SOCK_STREAM SHUT_RD",
>+ .run_client = test_stream_shutrd_client,
>+ .run_server = test_stream_shutrd_server,
>+ },
> {},
> };
>
>--
>2.25.1
>
On 02.08.2023 11:00, Stefano Garzarella wrote:
> On Tue, Aug 01, 2023 at 05:17:27PM +0300, Arseniy Krasnov wrote:
>> This adds two tests for 'shutdown()' call. It checks that SIGPIPE is
>> sent when MSG_NOSIGNAL is not set and vice versa. Both flags SHUT_WR
>> and SHUT_RD are tested.
>>
>> Signed-off-by: Arseniy Krasnov <[email protected]>
>> ---
>> tools/testing/vsock/vsock_test.c | 138 +++++++++++++++++++++++++++++++
>> 1 file changed, 138 insertions(+)
>>
>> diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
>> index 90718c2fd4ea..21d40a8d881c 100644
>> --- a/tools/testing/vsock/vsock_test.c
>> +++ b/tools/testing/vsock/vsock_test.c
>> @@ -19,6 +19,7 @@
>> #include <time.h>
>> #include <sys/mman.h>
>> #include <poll.h>
>> +#include <signal.h>
>>
>> #include "timeout.h"
>> #include "control.h"
>> @@ -1170,6 +1171,133 @@ static void test_seqpacket_msg_peek_server(const struct test_opts *opts)
>> return test_msg_peek_server(opts, true);
>> }
>>
>> +static bool have_sigpipe;
> ^
> We should define it as `volatile sig_atomic_t`:
>
> the behavior is undefined if the signal handler refers to any object
> [CX] [Option Start] other than errno [Option End] with static storage
> duration other than by assigning a value to an object declared as
> volatile sig_atomic_t
Yes, exactly, I forgot about sig_atomic_t.
I'll fix it.
Thanks, Arseniy
>
> https://pubs.opengroup.org/onlinepubs/9699919799/functions/signal.html
>
> The rest LGTM!
>
> Thanks,
> Stefano
>
>> +
>> +static void sigpipe(int signo)
>> +{
>> + have_sigpipe = true;
>> +}
>> +
>> +static void test_stream_check_sigpipe(int fd)
>> +{
>> + ssize_t res;
>> +
>> + have_sigpipe = false;
>> +
>> + res = send(fd, "A", 1, 0);
>> + if (res != -1) {
>> + fprintf(stderr, "expected send(2) failure, got %zi\n", res);
>> + exit(EXIT_FAILURE);
>> + }
>> +
>> + if (!have_sigpipe) {
>> + fprintf(stderr, "SIGPIPE expected\n");
>> + exit(EXIT_FAILURE);
>> + }
>> +
>> + have_sigpipe = false;
>> +
>> + res = send(fd, "A", 1, MSG_NOSIGNAL);
>> + if (res != -1) {
>> + fprintf(stderr, "expected send(2) failure, got %zi\n", res);
>> + exit(EXIT_FAILURE);
>> + }
>> +
>> + if (have_sigpipe) {
>> + fprintf(stderr, "SIGPIPE not expected\n");
>> + exit(EXIT_FAILURE);
>> + }
>> +}
>> +
>> +static void test_stream_shutwr_client(const struct test_opts *opts)
>> +{
>> + int fd;
>> +
>> + struct sigaction act = {
>> + .sa_handler = sigpipe,
>> + };
>> +
>> + sigaction(SIGPIPE, &act, NULL);
>> +
>> + fd = vsock_stream_connect(opts->peer_cid, 1234);
>> + if (fd < 0) {
>> + perror("connect");
>> + exit(EXIT_FAILURE);
>> + }
>> +
>> + if (shutdown(fd, SHUT_WR)) {
>> + perror("shutdown");
>> + exit(EXIT_FAILURE);
>> + }
>> +
>> + test_stream_check_sigpipe(fd);
>> +
>> + control_writeln("CLIENTDONE");
>> +
>> + close(fd);
>> +}
>> +
>> +static void test_stream_shutwr_server(const struct test_opts *opts)
>> +{
>> + int fd;
>> +
>> + fd = vsock_stream_accept(VMADDR_CID_ANY, 1234, NULL);
>> + if (fd < 0) {
>> + perror("accept");
>> + exit(EXIT_FAILURE);
>> + }
>> +
>> + control_expectln("CLIENTDONE");
>> +
>> + close(fd);
>> +}
>> +
>> +static void test_stream_shutrd_client(const struct test_opts *opts)
>> +{
>> + int fd;
>> +
>> + struct sigaction act = {
>> + .sa_handler = sigpipe,
>> + };
>> +
>> + sigaction(SIGPIPE, &act, NULL);
>> +
>> + fd = vsock_stream_connect(opts->peer_cid, 1234);
>> + if (fd < 0) {
>> + perror("connect");
>> + exit(EXIT_FAILURE);
>> + }
>> +
>> + control_expectln("SHUTRDDONE");
>> +
>> + test_stream_check_sigpipe(fd);
>> +
>> + control_writeln("CLIENTDONE");
>> +
>> + close(fd);
>> +}
>> +
>> +static void test_stream_shutrd_server(const struct test_opts *opts)
>> +{
>> + int fd;
>> +
>> + fd = vsock_stream_accept(VMADDR_CID_ANY, 1234, NULL);
>> + if (fd < 0) {
>> + perror("accept");
>> + exit(EXIT_FAILURE);
>> + }
>> +
>> + if (shutdown(fd, SHUT_RD)) {
>> + perror("shutdown");
>> + exit(EXIT_FAILURE);
>> + }
>> +
>> + control_writeln("SHUTRDDONE");
>> + control_expectln("CLIENTDONE");
>> +
>> + close(fd);
>> +}
>> +
>> static struct test_case test_cases[] = {
>> {
>> .name = "SOCK_STREAM connection reset",
>> @@ -1250,6 +1378,16 @@ static struct test_case test_cases[] = {
>> .run_client = test_seqpacket_msg_peek_client,
>> .run_server = test_seqpacket_msg_peek_server,
>> },
>> + {
>> + .name = "SOCK_STREAM SHUT_WR",
>> + .run_client = test_stream_shutwr_client,
>> + .run_server = test_stream_shutwr_server,
>> + },
>> + {
>> + .name = "SOCK_STREAM SHUT_RD",
>> + .run_client = test_stream_shutrd_client,
>> + .run_server = test_stream_shutrd_server,
>> + },
>> {},
>> };
>>
>> --
>> 2.25.1
>>
>