According to IEEE Std 1003.1, 2013, sending data to a SOCK_SEQPACKET
socketpair with MSG_NOSIGNAL flag set must result in a SIGPIPE if the
socket is no longer connected.
Signed-off-by: Alexander Potapenko <[email protected]>
---
I used the following program to check the kernel behavior:
/*****************/
#include <signal.h>
#include <stdio.h>
#include <string.h>
#include <sys/socket.h>
#include <unistd.h>
void PipeHandler(int sig) {
fprintf(stderr, "Killed by SIGPIPE\n");
_exit(1);
}
int main(int argc, char *argv[]) {
if (argc < 2) return 0;
struct sigaction act;
act.sa_handler = PipeHandler;
sigaction(SIGPIPE, &act, NULL);
int fds[2];
if (strcmp(argv[1], "seqpacket") == 0) {
if (socketpair(AF_UNIX, SOCK_SEQPACKET, 0, fds) == -1) return -1;
} else {
if (strcmp(argv[1], "stream") == 0) {
if (socketpair(AF_UNIX, SOCK_STREAM, 0, fds) == -1) return -1;
} else {
return -1;
}
}
int flags = 0;
if ((argc > 2) && (strcmp(argv[2], "nosignal") == 0))
flags = MSG_NOSIGNAL;
struct msghdr msg;
memset(&msg, 0, sizeof(msg));
close(fds[0]);
const ssize_t r = sendmsg(fds[1], &msg, flags);
if (r == -1) perror("sendmsg");
return 0;
}
/*****************/
Without the below patch the behavior is as follows:
$ ./sock seqpacket
sendmsg: Broken pipe
$ ./sock stream
Killed by SIGPIPE
$ ./sock seqpacket nosignal
sendmsg: Broken pipe
$ ./sock stream nosignal
sendmsg: Broken pipe
The behavior of the patched kernel complies with POSIX:
$ ./sock seqpacket
Killed by SIGPIPE
$ ./sock stream
Killed by SIGPIPE
$ ./sock seqpacket nosignal
sendmsg: Broken pipe
$ ./sock stream nosignal
sendmsg: Broken pipe
---
net/unix/af_unix.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 8fbe6d7..ba34c73 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1645,6 +1645,7 @@ static int unix_dgram_sendmsg(struct socket *sock, struct msghdr *msg,
struct sock *other = NULL;
int namelen = 0; /* fake GCC */
int err;
+ bool send_sigpipe = false;
unsigned int hash;
struct sk_buff *skb;
long timeo;
@@ -1675,6 +1676,12 @@ static int unix_dgram_sendmsg(struct socket *sock, struct msghdr *msg,
goto out;
}
+ if (sk->sk_shutdown & SEND_SHUTDOWN && sock->type == SOCK_SEQPACKET) {
+ send_sigpipe = true;
+ err = -EPIPE;
+ goto out;
+ }
+
if (test_bit(SOCK_PASSCRED, &sock->flags) && !u->addr
&& (err = unix_autobind(sock)) != 0)
goto out;
@@ -1769,8 +1776,11 @@ restart_locked:
}
err = -EPIPE;
- if (other->sk_shutdown & RCV_SHUTDOWN)
+ if (other->sk_shutdown & RCV_SHUTDOWN) {
+ if (sock->type == SOCK_SEQPACKET)
+ send_sigpipe = true;
goto out_unlock;
+ }
if (sk->sk_type != SOCK_SEQPACKET) {
err = security_unix_may_send(sk->sk_socket, other->sk_socket);
@@ -1837,6 +1847,8 @@ out:
if (other)
sock_put(other);
scm_destroy(&scm);
+ if (send_sigpipe && !(msg->msg_flags & MSG_NOSIGNAL))
+ send_sig(SIGPIPE, current, 0);
return err;
}
--
2.7.0.rc3.207.g0ac5344
From: Alexander Potapenko <[email protected]>
Date: Wed, 9 Mar 2016 15:10:23 +0100
> According to IEEE Std 1003.1, 2013, sending data to a SOCK_SEQPACKET
> socketpair with MSG_NOSIGNAL flag set must result in a SIGPIPE if the
> socket is no longer connected.
>
> Signed-off-by: Alexander Potapenko <[email protected]>
Please format your Subject line properly, the format is:
[PATCH] $SUBSYSTEM: $DESCRIPTION.
In this case "af_unx: " would be a proper subsystem marker.
Also,
> ---
> I used the following program to check the kernel behavior:
Please include this in your commit message rather than cutting it out
using the "---" marker, as it will help people in the future
understand your change and how you tested it. More information is
always better than less information.
Thanks.
Done, thanks for your response!
On Mon, Mar 14, 2016 at 8:19 PM, David Miller <[email protected]> wrote:
> From: Alexander Potapenko <[email protected]>
> Date: Wed, 9 Mar 2016 15:10:23 +0100
>
>> According to IEEE Std 1003.1, 2013, sending data to a SOCK_SEQPACKET
>> socketpair with MSG_NOSIGNAL flag set must result in a SIGPIPE if the
>> socket is no longer connected.
>>
>> Signed-off-by: Alexander Potapenko <[email protected]>
>
> Please format your Subject line properly, the format is:
>
> [PATCH] $SUBSYSTEM: $DESCRIPTION.
>
> In this case "af_unx: " would be a proper subsystem marker.
>
> Also,
>
>> ---
>> I used the following program to check the kernel behavior:
>
> Please include this in your commit message rather than cutting it out
> using the "---" marker, as it will help people in the future
> understand your change and how you tested it. More information is
> always better than less information.
>
> Thanks.
--
Alexander Potapenko
Software Engineer
Google Germany GmbH
Erika-Mann-Straße, 33
80636 München
Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg