From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
stable@vger.kernel.org, Daniel Borkmann <dborkman@redhat.com>,
Eric Dumazet <edumazet@google.com>,
"Eric W. Biederman" <ebiederm@xmission.com>,
"David S. Miller" <davem@davemloft.net>
Subject: [ 22/32] net: unix: inherit SOCK_PASS{CRED, SEC} flags from socket to fix race
Date: Fri, 1 Nov 2013 14:43:33 -0700 [thread overview]
Message-ID: <20131101214318.901862576@linuxfoundation.org> (raw)
In-Reply-To: <20131101214313.735463599@linuxfoundation.org>
3.4-stable review patch. If anyone has any objections, please let me know.
------------------
From: Daniel Borkmann <dborkman@redhat.com>
[ Upstream commit 90c6bd34f884cd9cee21f1d152baf6c18bcac949 ]
In the case of credentials passing in unix stream sockets (dgram
sockets seem not affected), we get a rather sparse race after
commit 16e5726 ("af_unix: dont send SCM_CREDENTIALS by default").
We have a stream server on receiver side that requests credential
passing from senders (e.g. nc -U). Since we need to set SO_PASSCRED
on each spawned/accepted socket on server side to 1 first (as it's
not inherited), it can happen that in the time between accept() and
setsockopt() we get interrupted, the sender is being scheduled and
continues with passing data to our receiver. At that time SO_PASSCRED
is neither set on sender nor receiver side, hence in cmsg's
SCM_CREDENTIALS we get eventually pid:0, uid:65534, gid:65534
(== overflow{u,g}id) instead of what we actually would like to see.
On the sender side, here nc -U, the tests in maybe_add_creds()
invoked through unix_stream_sendmsg() would fail, as at that exact
time, as mentioned, the sender has neither SO_PASSCRED on his side
nor sees it on the server side, and we have a valid 'other' socket
in place. Thus, sender believes it would just look like a normal
connection, not needing/requesting SO_PASSCRED at that time.
As reverting 16e5726 would not be an option due to the significant
performance regression reported when having creds always passed,
one way/trade-off to prevent that would be to set SO_PASSCRED on
the listener socket and allow inheriting these flags to the spawned
socket on server side in accept(). It seems also logical to do so
if we'd tell the listener socket to pass those flags onwards, and
would fix the race.
Before, strace:
recvmsg(4, {msg_name(0)=NULL, msg_iov(1)=[{"blub\n", 4096}],
msg_controllen=32, {cmsg_len=28, cmsg_level=SOL_SOCKET,
cmsg_type=SCM_CREDENTIALS{pid=0, uid=65534, gid=65534}},
msg_flags=0}, 0) = 5
After, strace:
recvmsg(4, {msg_name(0)=NULL, msg_iov(1)=[{"blub\n", 4096}],
msg_controllen=32, {cmsg_len=28, cmsg_level=SOL_SOCKET,
cmsg_type=SCM_CREDENTIALS{pid=11580, uid=1000, gid=1000}},
msg_flags=0}, 0) = 5
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
net/unix/af_unix.c | 10 ++++++++++
1 file changed, 10 insertions(+)
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1247,6 +1247,15 @@ static int unix_socketpair(struct socket
return 0;
}
+static void unix_sock_inherit_flags(const struct socket *old,
+ struct socket *new)
+{
+ if (test_bit(SOCK_PASSCRED, &old->flags))
+ set_bit(SOCK_PASSCRED, &new->flags);
+ if (test_bit(SOCK_PASSSEC, &old->flags))
+ set_bit(SOCK_PASSSEC, &new->flags);
+}
+
static int unix_accept(struct socket *sock, struct socket *newsock, int flags)
{
struct sock *sk = sock->sk;
@@ -1281,6 +1290,7 @@ static int unix_accept(struct socket *so
/* attach accepted sock to socket */
unix_state_lock(tsk);
newsock->state = SS_CONNECTED;
+ unix_sock_inherit_flags(sock, newsock);
sock_graft(tsk, newsock);
unix_state_unlock(tsk);
return 0;
next prev parent reply other threads:[~2013-11-01 21:45 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-11-01 21:43 [ 00/32] 3.4.68-stable review Greg Kroah-Hartman
2013-11-01 21:43 ` [ 01/32] tcp: must unclone packets before mangling them Greg Kroah-Hartman
2013-11-01 21:43 ` [ 02/32] tcp: do not forget FIN in tcp_shifted_skb() Greg Kroah-Hartman
2013-11-01 21:43 ` [ 03/32] net: do not call sock_put() on TIMEWAIT sockets Greg Kroah-Hartman
2013-11-01 21:43 ` [ 04/32] net: mv643xx_eth: update statistics timer from timer context only Greg Kroah-Hartman
2013-11-01 21:43 ` [ 05/32] net: mv643xx_eth: fix orphaned statistics timer crash Greg Kroah-Hartman
2013-11-01 21:43 ` [ 06/32] net: heap overflow in __audit_sockaddr() Greg Kroah-Hartman
2013-11-01 21:43 ` [ 07/32] proc connector: fix info leaks Greg Kroah-Hartman
2013-11-01 21:43 ` [ 08/32] ipv4: fix ineffective source address selection Greg Kroah-Hartman
2013-11-01 21:43 ` [ 09/32] can: dev: fix nlmsg size calculation in can_get_size() Greg Kroah-Hartman
2013-11-01 21:43 ` [ 10/32] ipv6: restrict neighbor entry creation to output flow Greg Kroah-Hartman
2013-11-01 21:43 ` [ 11/32] bridge: Correctly clamp MAX forward_delay when enabling STP Greg Kroah-Hartman
2013-11-01 21:43 ` [ 12/32] net: vlan: fix nlmsg size calculation in vlan_get_size() Greg Kroah-Hartman
2013-11-01 21:43 ` [ 13/32] l2tp: must disable bh before calling l2tp_xmit_skb() Greg Kroah-Hartman
2013-11-01 21:43 ` [ 14/32] farsync: fix info leak in ioctl Greg Kroah-Hartman
2013-11-01 21:43 ` [ 15/32] unix_diag: fix info leak Greg Kroah-Hartman
2013-11-01 21:43 ` [ 16/32] connector: use nlmsg_len() to check message length Greg Kroah-Hartman
2013-11-01 21:43 ` [ 17/32] bnx2x: record rx queue for LRO packets Greg Kroah-Hartman
2013-11-01 21:43 ` [ 18/32] net: dst: provide accessor function to dst->xfrm Greg Kroah-Hartman
2013-11-01 21:43 ` [ 19/32] sctp: Use software crc32 checksum when xfrm transform will happen Greg Kroah-Hartman
2013-11-01 21:43 ` [ 20/32] sctp: Perform software checksum if packet has to be fragmented Greg Kroah-Hartman
2013-11-01 21:43 ` [ 21/32] wanxl: fix info leak in ioctl Greg Kroah-Hartman
2013-11-01 21:43 ` Greg Kroah-Hartman [this message]
2013-11-01 21:43 ` [ 23/32] net: fix cipso packet validation when !NETLABEL Greg Kroah-Hartman
2013-11-01 21:43 ` [ 24/32] inet: fix possible memory corruption with UDP_CORK and UFO Greg Kroah-Hartman
2013-11-01 21:43 ` [ 25/32] davinci_emac.c: Fix IFF_ALLMULTI setup Greg Kroah-Hartman
2013-11-01 21:43 ` [ 26/32] ext3: return 32/64-bit dir name hash according to usage type Greg Kroah-Hartman
2013-11-01 21:43 ` [ 27/32] dm snapshot: fix data corruption Greg Kroah-Hartman
2013-11-01 21:43 ` [ 28/32] writeback: fix negative bdi max pause Greg Kroah-Hartman
2013-11-01 21:43 ` [ 29/32] wireless: radiotap: fix parsing buffer overrun Greg Kroah-Hartman
2013-11-01 21:43 ` [ 30/32] USB: serial: ti_usb_3410_5052: add Abbott strip port ID to combined table as well Greg Kroah-Hartman
2013-11-01 21:43 ` [ 31/32] USB: serial: option: add support for Inovia SEW858 device Greg Kroah-Hartman
2013-11-01 21:43 ` [ 32/32] usb: serial: option: blacklist Olivetti Olicard200 Greg Kroah-Hartman
2013-11-02 2:28 ` [ 00/32] 3.4.68-stable review Guenter Roeck
2013-11-02 21:33 ` Shuah Khan
2013-11-04 3:07 ` Satoru Takeuchi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20131101214318.901862576@linuxfoundation.org \
--to=gregkh@linuxfoundation.org \
--cc=davem@davemloft.net \
--cc=dborkman@redhat.com \
--cc=ebiederm@xmission.com \
--cc=edumazet@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox