netdev - Re: [PATCH] af_unix: Only allow recv on connected seqpacket sockets.

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20110424.120519.226767465.davem@davemloft.net>
Date:	Sun, 24 Apr 2011 12:05:19 -0700 (PDT)
From:	David Miller <davem@...emloft.net>
To:	ebiederm@...ssion.com
Cc:	netdev@...r.kernel.org, xemul@...nvz.org, dan@...ni.org,
	stable@...nel.org
Subject: Re: [PATCH] af_unix: Only allow recv on connected seqpacket
 sockets.

From: ebiederm@...ssion.com (Eric W. Biederman)
Date: Sun, 24 Apr 2011 04:54:57 -0700

> +static int unix_seqpacket_recvmsg(struct kiocb *iocb, struct socket *sock,
> +			      struct msghdr *msg, size_t size,
> +			      int flags)
> +{
> +	struct sock *sk = sock->sk;
> +
> +	if (sk->sk_state != TCP_ESTABLISHED)
> +		return -ENOTCONN;

As for unix_seqpacket_sendmsg(), you need to add a check for sock_error()
or similar here otherwise -ECONNRESET is not reported correctly.

In fact, recvmsg() is even harder than sendmsg() to handle correctly,
because we have to also properly report EOF on seqpacket sockets which
have RCV_SHUTDOWN set.

So a lot more work has to go into this change to make it fix the bug
without also breaking existing semantics.

Anyways, see:

commit 6e14891f4d16f8a9e0bc3a8408f73b3aed93ab0a
Author: James Morris <jmorris@...hat.com>
Date:   Fri Nov 19 07:02:41 2004 -0800

    [AF_UNIX]: Don't lose ECONNRESET in unix_seqpacket_sendmsg()
    
    The fix for SELinux w/SOCK_SEQPACKET had an error,
    noted by Alan Cox.  This fixes it.
    
    Signed-off-by: James Morris <jmorris@...hat.com>
    Signed-off-by: David S. Miller <davem@...emloft.net>

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 16faa9d..8902c4a 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1513,13 +1513,18 @@ out_err:
 static int unix_seqpacket_sendmsg(struct kiocb *kiocb, struct socket *sock,
 				  struct msghdr *msg, size_t len)
 {
+	int err;
 	struct sock *sk = sock->sk;
 	
+	err = sock_error(sk);
+	if (err)
+		return err;
+
 	if (sk->sk_state != TCP_ESTABLISHED)
 		return -ENOTCONN;
 
-	if (msg->msg_name || msg->msg_namelen)
-		return -EINVAL;
+	if (msg->msg_namelen)
+		msg->msg_namelen = 0;
 
 	return unix_dgram_sendmsg(kiocb, sock, msg, len);
 }
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html