From patchwork Tue Jul 25 20:45:13 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Amit Pundir X-Patchwork-Id: 108698 Delivered-To: patch@linaro.org Received: by 10.140.101.44 with SMTP id t41csp28738qge; Tue, 25 Jul 2017 13:45:44 -0700 (PDT) X-Received: by 10.84.209.232 with SMTP id y95mr22692154plh.391.1501015544348; Tue, 25 Jul 2017 13:45:44 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1501015544; cv=none; d=google.com; s=arc-20160816; b=TCaYlgF4ndy7oXe2yYU+Bc1MLhGf+CiXXi4vt9hYPKCVMUDrcbf887KK89yvBdGXF1 iVxqA5zgGn6ggKdHWQv56T1wZ2kPLdFKaJnnQd6nPfvAEc0dCh9DEixuujxGVTDBFvH5 BM+w/nx+DDJAyQoMt7UyuMaIIvH/ugs76DJzReJIk3nkbW34YZRYVkLQ7ZTQH8fle3D3 V/thLSkBVQXv6wpw1ivNWaj539JhAEjWKnK+rhSb1VnoNHpq9CBrobWMksC2M1vLMB63 ErdcHXD5Z/LetEKLoOVVwVAv1rp22sn4nriqDj1xLkGO8JB1ohXeBYaiFCod24bXFnff hEog== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:references:in-reply-to:message-id:date :subject:cc:to:from:dkim-signature:arc-authentication-results; bh=pKRvgK0/rSDft9SKEmeRMEExdO2pOqRywP/i7j4a2Ws=; b=jwbhff799B8EeLz0tRjgGGEknhmwQNwxv9I9TjMpvuZYypW1zwLSOXandtcTn4xH6w ZkDc0/QQVjnfrKKo7jyi/UxapEh4Ha0pepAg6OSASjMq+85Xhi5If65GReiQjXaHMDkd S89yOCdtm4Siv8CiKX/aTAKtF5xAHkjkVn0uX/oJ+WDUER5WLEdxMlUJBncQ7o5MyVND Ma2Hlp2R7U4KFOn/4zp9ydEj5WXgQUZc6B5sRZmHZUpv3Wy0RVC2kwWbsgMg7DGPWLTz hqVEFqxbK2G/WSWgbpiyzbM8X7ufnQoQWHJiDsceAhAWOF5elaBzPTc0NIAUlMBcstMx otdw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.b=P8JSulEQ; spf=pass (google.com: best guess record for domain of stable-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=stable-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id u1si9102153plm.762.2017.07.25.13.45.44; Tue, 25 Jul 2017 13:45:44 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of stable-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.b=P8JSulEQ; spf=pass (google.com: best guess record for domain of stable-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=stable-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752049AbdGYUpk (ORCPT + 6 others); Tue, 25 Jul 2017 16:45:40 -0400 Received: from mail-pg0-f46.google.com ([74.125.83.46]:36809 "EHLO mail-pg0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751730AbdGYUph (ORCPT ); Tue, 25 Jul 2017 16:45:37 -0400 Received: by mail-pg0-f46.google.com with SMTP id 125so74831014pgi.3 for ; Tue, 25 Jul 2017 13:45:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=pKRvgK0/rSDft9SKEmeRMEExdO2pOqRywP/i7j4a2Ws=; b=P8JSulEQQIRTPkkPDFhTlIVg7t+kyKQBVWSZPz4iiUF1yxuAJoq3ATut04apI5ZmSw IJqPpMwg9jJM9/TxmYqoDffNmjnbQE1I6Ai4gfzbSDcriPD2yMOYXeVsFibzWSVKEj6M bpPWU8661mZR7Pm+ZzSri7VWlng0uOvfmsMcc= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=pKRvgK0/rSDft9SKEmeRMEExdO2pOqRywP/i7j4a2Ws=; b=ny+cOrvGuCfvMAqQe/xUIhX/n3lkMpeOaNAMqc3pj7u+arUSoYIydN6SUfZ6tOAe+5 PM3AvVFfRhIiv7fum5uOFUpDc1mourxN4JS1omDo980JW9J1TiX/24SCG8PFUTfgqGZt OP/prdCL+wVI19yvdmDRfNkQ0K4DVF9tmf5muBziC643O/nLUO5YCynDnsNgYGXXO9pJ 1mze4gm/1kl4rBKK+KCkJLHe/diP6C4yC6qiIB7qwhs/1uIlzFaON9XFceVrlsqu/xiS 8dzep6NpJ48EoNp4qfjldX2iSyhseeMmeEL11YNrsHuMUxhc9e1aTJM6wHUHXsLuVlQe XgPA== X-Gm-Message-State: AIVw112c0tcu75D1LEjtwZb2RXLvl9c41JIVWq7swo7ArhDoWQq89FUI ccZVL3ZsPVEHwkEv X-Received: by 10.98.63.10 with SMTP id m10mr11371967pfa.232.1501015537004; Tue, 25 Jul 2017 13:45:37 -0700 (PDT) Received: from localhost.localdomain ([106.51.135.235]) by smtp.gmail.com with ESMTPSA id d4sm532125pfj.59.2017.07.25.13.45.34 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Tue, 25 Jul 2017 13:45:35 -0700 (PDT) From: Amit Pundir To: Greg KH Cc: Stable , Daniel Borkmann , "David S . Miller" Subject: [PATCH for-3.18 02/15] net: sctp: fix race for one-to-many sockets in sendmsg's auto associate Date: Wed, 26 Jul 2017 02:15:13 +0530 Message-Id: <1501015526-32178-3-git-send-email-amit.pundir@linaro.org> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1501015526-32178-1-git-send-email-amit.pundir@linaro.org> References: <1501015526-32178-1-git-send-email-amit.pundir@linaro.org> Sender: stable-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org 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(8888), 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(8888), 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(8888), 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(8888), 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(8888), 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.8888: sctp (1) [INIT] [init tag: 3879023686] [rwnd: 106496] [OS: 10] [MIS: 65535] [init TSN: 3139201684] 22:33:36.316619 IP 192.168.1.115.8888 > 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.8888: sctp (1) [ABORT] tcpdump after patch: 14:28:58.884116 IP 192.168.1.114.35846 > 192.168.1.115.8888: sctp (1) [INIT] [init tag: 438593213] [rwnd: 106496] [OS: 10] [MIS: 65535] [init TSN: 3092969729] 14:28:58.888414 IP 192.168.1.115.8888 > 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.8888: sctp (1) [COOKIE ECHO] , (2) [DATA] (B)(E) [TSN: 3092969729] [...] 14:28:58.893278 IP 192.168.1.115.8888 > 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.8888: sctp (1) [DATA] (B)(E) [TSN: 3092969730] [...] 14:28:59.096963 IP 192.168.1.115.8888 > 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.8888: sctp (1) [DATA] (B)(E) [TSN: 3092969731] [...] , (2) [DATA] (B)(E) [TSN: 3092969732] [...] 14:28:59.103218 IP 192.168.1.115.8888 > 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.8888: sctp (1) [SHUTDOWN] 14:28:59.107793 IP 192.168.1.115.8888 > 192.168.1.114.35846: sctp (1) [SHUTDOWN ACK] 14:28:59.107890 IP 192.168.1.114.35846 > 192.168.1.115.8888: 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: David S. Miller Signed-off-by: Amit Pundir --- net/sctp/socket.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) -- 2.7.4 diff --git a/net/sctp/socket.c b/net/sctp/socket.c index 92c920c9cfa6..92c6eac72ea6 100644 --- a/net/sctp/socket.c +++ b/net/sctp/socket.c @@ -1604,7 +1604,7 @@ static int sctp_sendmsg(struct kiocb *iocb, struct sock *sk, sctp_assoc_t associd = 0; sctp_cmsgs_t cmsgs = { NULL }; sctp_scope_t scope; - bool fill_sinfo_ttl = false; + bool fill_sinfo_ttl = false, wait_connect = false; struct sctp_datamsg *datamsg; int msg_flags = msg->msg_flags; __u16 sinfo_flags = 0; @@ -1944,6 +1944,7 @@ static int sctp_sendmsg(struct kiocb *iocb, struct sock *sk, if (err < 0) goto out_free; + wait_connect = true; pr_debug("%s: we associated primitively\n", __func__); } @@ -1981,6 +1982,11 @@ static int sctp_sendmsg(struct kiocb *iocb, struct sock *sk, sctp_datamsg_put(datamsg); err = msg_len; + if (unlikely(wait_connect)) { + timeo = sock_sndtimeo(sk, msg_flags & MSG_DONTWAIT); + sctp_wait_for_connect(asoc, &timeo); + } + /* If we are already past ASSOCIATE, the lower * layers are responsible for association cleanup. */