[PATCH 3.18 28/92] net: sctp: fix race for one-to-many sockets in sendmsgs auto associate

2017-08-09 Thread Greg Kroah-Hartman
3.18-stable review patch.  If anyone has any objections, please let me know.

--

From: Daniel Borkmann 

commit 2061dcd6bff8b774b4fac8b0739b6be3f87bc9f2 upstream.

I.e. one-to-many sockets in SCTP are not required to explicitly
call into connect(2) or sctp_connectx(2) prior to data exchange.
Instead, they can directly invoke sendmsg(2) and the SCTP stack
will automatically trigger connection establishment through 4WHS
via sctp_primitive_ASSOCIATE(). However, this in its current
implementation is racy: INIT is being sent out immediately (as
it cannot be bundled anyway) and the rest of the DATA chunks are
queued up for later xmit when connection is established, meaning
sendmsg(2) will return successfully. This behaviour can result
in an undesired side-effect that the kernel made the application
think the data has already been transmitted, although none of it
has actually left the machine, worst case even after close(2)'ing
the socket.

Instead, when the association from client side has been shut down
e.g. first gracefully through SCTP_EOF and then close(2), the
client could afterwards still receive the server's INIT_ACK due
to a connection with higher latency. This INIT_ACK is then considered
out of the blue and hence responded with ABORT as there was no
alive assoc found anymore. This can be easily reproduced f.e.
with sctp_test application from lksctp. One way to fix this race
is to wait for the handshake to actually complete.

The fix defers waiting after sctp_primitive_ASSOCIATE() and
sctp_primitive_SEND() succeeded, so that DATA chunks cooked up
from sctp_sendmsg() have already been placed into the output
queue through the side-effect interpreter, and therefore can then
be bundeled together with COOKIE_ECHO control chunks.

strace from example application (shortened):

socket(PF_INET, SOCK_SEQPACKET, IPPROTO_SCTP) = 3
sendmsg(3, {msg_name(28)={sa_family=AF_INET, sin_port=htons(), 
sin_addr=inet_addr("192.168.1.115")},
   msg_iov(1)=[{"hello", 5}], msg_controllen=0, msg_flags=0}, 0) = 5
sendmsg(3, {msg_name(28)={sa_family=AF_INET, sin_port=htons(), 
sin_addr=inet_addr("192.168.1.115")},
   msg_iov(1)=[{"hello", 5}], msg_controllen=0, msg_flags=0}, 0) = 5
sendmsg(3, {msg_name(28)={sa_family=AF_INET, sin_port=htons(), 
sin_addr=inet_addr("192.168.1.115")},
   msg_iov(1)=[{"hello", 5}], msg_controllen=0, msg_flags=0}, 0) = 5
sendmsg(3, {msg_name(28)={sa_family=AF_INET, sin_port=htons(), 
sin_addr=inet_addr("192.168.1.115")},
   msg_iov(1)=[{"hello", 5}], msg_controllen=0, msg_flags=0}, 0) = 5
sendmsg(3, {msg_name(28)={sa_family=AF_INET, sin_port=htons(), 
sin_addr=inet_addr("192.168.1.115")},
   msg_iov(0)=[], msg_controllen=48, {cmsg_len=48, cmsg_level=0x84 /* 
SOL_??? */, cmsg_type=, ...},
   msg_flags=0}, 0) = 0 // graceful shutdown for SOCK_SEQPACKET via 
SCTP_EOF
close(3) = 0

tcpdump before patch (fooling the application):

22:33:36.306142 IP 192.168.1.114.41462 > 192.168.1.115.: sctp (1) [INIT] 
[init tag: 3879023686] [rwnd: 106496] [OS: 10] [MIS: 65535] [init TSN: 
3139201684]
22:33:36.316619 IP 192.168.1.115. > 192.168.1.114.41462: sctp (1) [INIT 
ACK] [init tag: 3345394793] [rwnd: 106496] [OS: 10] [MIS: 10] [init TSN: 
3380109591]
22:33:36.317600 IP 192.168.1.114.41462 > 192.168.1.115.: sctp (1) [ABORT]

tcpdump after patch:

14:28:58.884116 IP 192.168.1.114.35846 > 192.168.1.115.: sctp (1) [INIT] 
[init tag: 438593213] [rwnd: 106496] [OS: 10] [MIS: 65535] [init TSN: 
3092969729]
14:28:58.888414 IP 192.168.1.115. > 192.168.1.114.35846: sctp (1) [INIT 
ACK] [init tag: 381429855] [rwnd: 106496] [OS: 10] [MIS: 10] [init TSN: 
2141904492]
14:28:58.888638 IP 192.168.1.114.35846 > 192.168.1.115.: sctp (1) [COOKIE 
ECHO] , (2) [DATA] (B)(E) [TSN: 3092969729] [...]
14:28:58.893278 IP 192.168.1.115. > 192.168.1.114.35846: sctp (1) [COOKIE 
ACK] , (2) [SACK] [cum ack 3092969729] [a_rwnd 106491] [#gap acks 0] [#dup tsns 
0]
14:28:58.893591 IP 192.168.1.114.35846 > 192.168.1.115.: sctp (1) [DATA] 
(B)(E) [TSN: 3092969730] [...]
14:28:59.096963 IP 192.168.1.115. > 192.168.1.114.35846: sctp (1) [SACK] 
[cum ack 3092969730] [a_rwnd 106496] [#gap acks 0] [#dup tsns 0]
14:28:59.097086 IP 192.168.1.114.35846 > 192.168.1.115.: sctp (1) [DATA] 
(B)(E) [TSN: 3092969731] [...] , (2) [DATA] (B)(E) [TSN: 3092969732] [...]
14:28:59.103218 IP 192.168.1.115. > 192.168.1.114.35846: sctp (1) [SACK] 
[cum ack 3092969732] [a_rwnd 106486] [#gap acks 0] [#dup tsns 0]
14:28:59.103330 IP 192.168.1.114.35846 > 192.168.1.115.: sctp (1) [SHUTDOWN]
14:28:59.107793 IP 192.168.1.115. > 192.168.1.114.35846: sctp (1) [SHUTDOWN 
ACK]
14:28:59.107890 IP 192.168.1.114.35846 > 192.168.1.115.: sctp (1) [SHUTDOWN 
COMPLETE]

Looks like this bug is from the pre-git history museum. ;)

Fixes: 08707d5482df ("lksctp-2_5_31-0_5_1.patch")
Signed-off-by: Daniel Borkmann 

[PATCH 3.18 28/92] net: sctp: fix race for one-to-many sockets in sendmsgs auto associate

2017-08-09 Thread Greg Kroah-Hartman
3.18-stable review patch.  If anyone has any objections, please let me know.

--

From: Daniel Borkmann 

commit 2061dcd6bff8b774b4fac8b0739b6be3f87bc9f2 upstream.

I.e. one-to-many sockets in SCTP are not required to explicitly
call into connect(2) or sctp_connectx(2) prior to data exchange.
Instead, they can directly invoke sendmsg(2) and the SCTP stack
will automatically trigger connection establishment through 4WHS
via sctp_primitive_ASSOCIATE(). However, this in its current
implementation is racy: INIT is being sent out immediately (as
it cannot be bundled anyway) and the rest of the DATA chunks are
queued up for later xmit when connection is established, meaning
sendmsg(2) will return successfully. This behaviour can result
in an undesired side-effect that the kernel made the application
think the data has already been transmitted, although none of it
has actually left the machine, worst case even after close(2)'ing
the socket.

Instead, when the association from client side has been shut down
e.g. first gracefully through SCTP_EOF and then close(2), the
client could afterwards still receive the server's INIT_ACK due
to a connection with higher latency. This INIT_ACK is then considered
out of the blue and hence responded with ABORT as there was no
alive assoc found anymore. This can be easily reproduced f.e.
with sctp_test application from lksctp. One way to fix this race
is to wait for the handshake to actually complete.

The fix defers waiting after sctp_primitive_ASSOCIATE() and
sctp_primitive_SEND() succeeded, so that DATA chunks cooked up
from sctp_sendmsg() have already been placed into the output
queue through the side-effect interpreter, and therefore can then
be bundeled together with COOKIE_ECHO control chunks.

strace from example application (shortened):

socket(PF_INET, SOCK_SEQPACKET, IPPROTO_SCTP) = 3
sendmsg(3, {msg_name(28)={sa_family=AF_INET, sin_port=htons(), 
sin_addr=inet_addr("192.168.1.115")},
   msg_iov(1)=[{"hello", 5}], msg_controllen=0, msg_flags=0}, 0) = 5
sendmsg(3, {msg_name(28)={sa_family=AF_INET, sin_port=htons(), 
sin_addr=inet_addr("192.168.1.115")},
   msg_iov(1)=[{"hello", 5}], msg_controllen=0, msg_flags=0}, 0) = 5
sendmsg(3, {msg_name(28)={sa_family=AF_INET, sin_port=htons(), 
sin_addr=inet_addr("192.168.1.115")},
   msg_iov(1)=[{"hello", 5}], msg_controllen=0, msg_flags=0}, 0) = 5
sendmsg(3, {msg_name(28)={sa_family=AF_INET, sin_port=htons(), 
sin_addr=inet_addr("192.168.1.115")},
   msg_iov(1)=[{"hello", 5}], msg_controllen=0, msg_flags=0}, 0) = 5
sendmsg(3, {msg_name(28)={sa_family=AF_INET, sin_port=htons(), 
sin_addr=inet_addr("192.168.1.115")},
   msg_iov(0)=[], msg_controllen=48, {cmsg_len=48, cmsg_level=0x84 /* 
SOL_??? */, cmsg_type=, ...},
   msg_flags=0}, 0) = 0 // graceful shutdown for SOCK_SEQPACKET via 
SCTP_EOF
close(3) = 0

tcpdump before patch (fooling the application):

22:33:36.306142 IP 192.168.1.114.41462 > 192.168.1.115.: sctp (1) [INIT] 
[init tag: 3879023686] [rwnd: 106496] [OS: 10] [MIS: 65535] [init TSN: 
3139201684]
22:33:36.316619 IP 192.168.1.115. > 192.168.1.114.41462: sctp (1) [INIT 
ACK] [init tag: 3345394793] [rwnd: 106496] [OS: 10] [MIS: 10] [init TSN: 
3380109591]
22:33:36.317600 IP 192.168.1.114.41462 > 192.168.1.115.: sctp (1) [ABORT]

tcpdump after patch:

14:28:58.884116 IP 192.168.1.114.35846 > 192.168.1.115.: sctp (1) [INIT] 
[init tag: 438593213] [rwnd: 106496] [OS: 10] [MIS: 65535] [init TSN: 
3092969729]
14:28:58.888414 IP 192.168.1.115. > 192.168.1.114.35846: sctp (1) [INIT 
ACK] [init tag: 381429855] [rwnd: 106496] [OS: 10] [MIS: 10] [init TSN: 
2141904492]
14:28:58.888638 IP 192.168.1.114.35846 > 192.168.1.115.: sctp (1) [COOKIE 
ECHO] , (2) [DATA] (B)(E) [TSN: 3092969729] [...]
14:28:58.893278 IP 192.168.1.115. > 192.168.1.114.35846: sctp (1) [COOKIE 
ACK] , (2) [SACK] [cum ack 3092969729] [a_rwnd 106491] [#gap acks 0] [#dup tsns 
0]
14:28:58.893591 IP 192.168.1.114.35846 > 192.168.1.115.: sctp (1) [DATA] 
(B)(E) [TSN: 3092969730] [...]
14:28:59.096963 IP 192.168.1.115. > 192.168.1.114.35846: sctp (1) [SACK] 
[cum ack 3092969730] [a_rwnd 106496] [#gap acks 0] [#dup tsns 0]
14:28:59.097086 IP 192.168.1.114.35846 > 192.168.1.115.: sctp (1) [DATA] 
(B)(E) [TSN: 3092969731] [...] , (2) [DATA] (B)(E) [TSN: 3092969732] [...]
14:28:59.103218 IP 192.168.1.115. > 192.168.1.114.35846: sctp (1) [SACK] 
[cum ack 3092969732] [a_rwnd 106486] [#gap acks 0] [#dup tsns 0]
14:28:59.103330 IP 192.168.1.114.35846 > 192.168.1.115.: sctp (1) [SHUTDOWN]
14:28:59.107793 IP 192.168.1.115. > 192.168.1.114.35846: sctp (1) [SHUTDOWN 
ACK]
14:28:59.107890 IP 192.168.1.114.35846 > 192.168.1.115.: sctp (1) [SHUTDOWN 
COMPLETE]

Looks like this bug is from the pre-git history museum. ;)

Fixes: 08707d5482df ("lksctp-2_5_31-0_5_1.patch")
Signed-off-by: Daniel Borkmann 
Acked-by: Vlad Yasevich 
Signed-off-by: