linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] SOCK_SEQPACKET socketpair must get SIGPIPE in AF_UNIX if one end is closed
@ 2016-03-09 14:10 Alexander Potapenko
  2016-03-14 19:19 ` David Miller
  0 siblings, 1 reply; 3+ messages in thread
From: Alexander Potapenko @ 2016-03-09 14:10 UTC (permalink / raw)
  To: edumazet, rweikusat; +Cc: linux-kernel, netdev

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 <glider@google.com>
---
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

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] SOCK_SEQPACKET socketpair must get SIGPIPE in AF_UNIX if one end is closed
  2016-03-09 14:10 [PATCH] SOCK_SEQPACKET socketpair must get SIGPIPE in AF_UNIX if one end is closed Alexander Potapenko
@ 2016-03-14 19:19 ` David Miller
  2016-03-15  9:08   ` Alexander Potapenko
  0 siblings, 1 reply; 3+ messages in thread
From: David Miller @ 2016-03-14 19:19 UTC (permalink / raw)
  To: glider; +Cc: edumazet, rweikusat, linux-kernel, netdev

From: Alexander Potapenko <glider@google.com>
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 <glider@google.com>

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.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] SOCK_SEQPACKET socketpair must get SIGPIPE in AF_UNIX if one end is closed
  2016-03-14 19:19 ` David Miller
@ 2016-03-15  9:08   ` Alexander Potapenko
  0 siblings, 0 replies; 3+ messages in thread
From: Alexander Potapenko @ 2016-03-15  9:08 UTC (permalink / raw)
  To: David Miller; +Cc: Eric Dumazet, rweikusat, LKML, netdev

Done, thanks for your response!

On Mon, Mar 14, 2016 at 8:19 PM, David Miller <davem@davemloft.net> wrote:
> From: Alexander Potapenko <glider@google.com>
> 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 <glider@google.com>
>
> 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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-03-15  9:09 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-09 14:10 [PATCH] SOCK_SEQPACKET socketpair must get SIGPIPE in AF_UNIX if one end is closed Alexander Potapenko
2016-03-14 19:19 ` David Miller
2016-03-15  9:08   ` Alexander Potapenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).