stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Vegard Nossum <vegard.nossum@oracle.com>,
	Wei Wang <weiwan@google.com>, Eric Dumazet <edumazet@google.com>,
	"David S. Miller" <davem@davemloft.net>
Subject: [PATCH 3.18 15/33] tcp: avoid fastopen API to be used on AF_UNSPEC
Date: Mon,  5 Jun 2017 18:17:08 +0200	[thread overview]
Message-ID: <20170605153021.550152891@linuxfoundation.org> (raw)
In-Reply-To: <20170605153020.953645204@linuxfoundation.org>

3.18-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Wei Wang <weiwan@google.com>


[ Upstream commit ba615f675281d76fd19aa03558777f81fb6b6084 ]

Fastopen API should be used to perform fastopen operations on the TCP
socket. It does not make sense to use fastopen API to perform disconnect
by calling it with AF_UNSPEC. The fastopen data path is also prone to
race conditions and bugs when using with AF_UNSPEC.

One issue reported and analyzed by Vegard Nossum is as follows:
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Thread A:                            Thread B:
------------------------------------------------------------------------
sendto()
 - tcp_sendmsg()
     - sk_stream_memory_free() = 0
         - goto wait_for_sndbuf
	     - sk_stream_wait_memory()
	        - sk_wait_event() // sleep
          |                          sendto(flags=MSG_FASTOPEN, dest_addr=AF_UNSPEC)
	  |                           - tcp_sendmsg()
	  |                              - tcp_sendmsg_fastopen()
	  |                                 - __inet_stream_connect()
	  |                                    - tcp_disconnect() //because of AF_UNSPEC
	  |                                       - tcp_transmit_skb()// send RST
	  |                                    - return 0; // no reconnect!
	  |                           - sk_stream_wait_connect()
	  |                                 - sock_error()
	  |                                    - xchg(&sk->sk_err, 0)
	  |                                    - return -ECONNRESET
	- ... // wake up, see sk->sk_err == 0
    - skb_entail() on TCP_CLOSE socket

If the connection is reopened then we will send a brand new SYN packet
after thread A has already queued a buffer. At this point I think the
socket internal state (sequence numbers etc.) becomes messed up.

When the new connection is closed, the FIN-ACK is rejected because the
sequence number is outside the window. The other side tries to
retransmit,
but __tcp_retransmit_skb() calls tcp_trim_head() on an empty skb which
corrupts the skb data length and hits a BUG() in copy_and_csum_bits().
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++

Hence, this patch adds a check for AF_UNSPEC in the fastopen data path
and return EOPNOTSUPP to user if such case happens.

Fixes: cf60af03ca4e7 ("tcp: Fast Open client - sendmsg(MSG_FASTOPEN)")
Reported-by: Vegard Nossum <vegard.nossum@oracle.com>
Signed-off-by: Wei Wang <weiwan@google.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 net/ipv4/tcp.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1066,9 +1066,12 @@ static int tcp_sendmsg_fastopen(struct s
 				int *copied, size_t size)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
+	struct sockaddr *uaddr = msg->msg_name;
 	int err, flags;
 
-	if (!(sysctl_tcp_fastopen & TFO_CLIENT_ENABLE))
+	if (!(sysctl_tcp_fastopen & TFO_CLIENT_ENABLE) ||
+	    (uaddr && msg->msg_namelen >= sizeof(uaddr->sa_family) &&
+	     uaddr->sa_family == AF_UNSPEC))
 		return -EOPNOTSUPP;
 	if (tp->fastopen_req != NULL)
 		return -EALREADY; /* Another Fast Open is in progress */
@@ -1081,7 +1084,7 @@ static int tcp_sendmsg_fastopen(struct s
 	tp->fastopen_req->size = size;
 
 	flags = (msg->msg_flags & MSG_DONTWAIT) ? O_NONBLOCK : 0;
-	err = __inet_stream_connect(sk->sk_socket, msg->msg_name,
+	err = __inet_stream_connect(sk->sk_socket, uaddr,
 				    msg->msg_namelen, flags);
 	*copied = tp->fastopen_req->copied;
 	tcp_free_fastopen_req(tp);

  parent reply	other threads:[~2017-06-05 16:17 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-05 16:16 [PATCH 3.18 00/33] 3.18.56-stable review Greg Kroah-Hartman
2017-06-05 16:16 ` [PATCH 3.18 02/33] netem: fix skb_orphan_partial() Greg Kroah-Hartman
2017-06-05 16:16 ` [PATCH 3.18 03/33] dccp/tcp: do not inherit mc_list from parent Greg Kroah-Hartman
2017-06-05 16:16 ` [PATCH 3.18 04/33] s390/qeth: handle sysfs error during initialization Greg Kroah-Hartman
2017-06-05 16:16 ` [PATCH 3.18 05/33] s390/qeth: unbreak OSM and OSN support Greg Kroah-Hartman
2017-06-05 16:16 ` [PATCH 3.18 06/33] s390/qeth: avoid null pointer dereference on OSN Greg Kroah-Hartman
2017-06-05 16:17 ` [PATCH 3.18 07/33] tcp: avoid fragmenting peculiar skbs in SACK Greg Kroah-Hartman
2017-06-05 16:17 ` [PATCH 3.18 08/33] sctp: fix src address selection if using secondary addresses for ipv6 Greg Kroah-Hartman
2017-06-05 16:17 ` [PATCH 3.18 09/33] sctp: do not inherit ipv6_{mc|ac|fl}_list from parent Greg Kroah-Hartman
2017-06-05 16:17 ` [PATCH 3.18 10/33] tcp: eliminate negative reordering in tcp_clean_rtx_queue Greg Kroah-Hartman
2017-06-05 16:17 ` [PATCH 3.18 11/33] ipv6: Prevent overrun when parsing v6 header options Greg Kroah-Hartman
2017-06-05 16:17 ` [PATCH 3.18 12/33] ipv6: Check ip6_find_1stfragopt() return value properly Greg Kroah-Hartman
2017-06-05 16:17 ` [PATCH 3.18 14/33] ipv6: fix out of bound writes in __ip6_append_data() Greg Kroah-Hartman
2017-06-05 16:17 ` Greg Kroah-Hartman [this message]
2017-06-05 16:17 ` [PATCH 3.18 16/33] sctp: fix ICMP processing if skb is non-linear Greg Kroah-Hartman
2017-06-05 16:17 ` [PATCH 3.18 17/33] i2c: i2c-tiny-usb: fix buffer not being DMA capable Greg Kroah-Hartman
2017-06-05 16:17 ` [PATCH 3.18 19/33] pcmcia: remove left-over %Z format Greg Kroah-Hartman
2017-06-05 16:17 ` [PATCH 3.18 20/33] ALSA: hda - apply STAC_9200_DELL_M22 quirk for Dell Latitude D430 Greg Kroah-Hartman
2017-06-05 16:17 ` [PATCH 3.18 21/33] slub/memcg: cure the brainless abuse of sysfs attributes Greg Kroah-Hartman
2017-06-05 16:17 ` [PATCH 3.18 22/33] drm/gma500/psb: Actually use VBT mode when it is found Greg Kroah-Hartman
2017-06-05 16:17 ` [PATCH 3.18 23/33] mm/migrate: fix refcount handling when !hugepage_migration_supported() Greg Kroah-Hartman
2017-06-05 16:17 ` [PATCH 3.18 24/33] mlock: fix mlock count can not decrease in race condition Greg Kroah-Hartman
2017-06-05 16:17 ` [PATCH 3.18 25/33] xfs: Fix missed holes in SEEK_HOLE implementation Greg Kroah-Hartman
2017-06-05 16:17 ` [PATCH 3.18 26/33] xfs: fix off-by-one on max nr_pages in xfs_find_get_desired_pgoff() Greg Kroah-Hartman
2017-06-05 16:17 ` [PATCH 3.18 27/33] xfs: fix over-copying of getbmap parameters from userspace Greg Kroah-Hartman
2017-06-05 16:17 ` [PATCH 3.18 28/33] xfs: handle array index overrun in xfs_dir2_leaf_readbuf() Greg Kroah-Hartman
2017-06-05 16:17 ` [PATCH 3.18 29/33] xfs: prevent multi-fsb dir readahead from reading random blocks Greg Kroah-Hartman
2017-06-05 16:17 ` [PATCH 3.18 30/33] xfs: fix up quotacheck buffer list error handling Greg Kroah-Hartman
2017-06-05 16:17 ` [PATCH 3.18 31/33] xfs: fix indlen accounting error on partial delalloc conversion Greg Kroah-Hartman
2017-06-05 16:17 ` [PATCH 3.18 32/33] xfs: bad assertion for delalloc an extent that start at i_size Greg Kroah-Hartman
2017-06-05 16:17 ` [PATCH 3.18 33/33] xfs: fix unaligned access in xfs_btree_visit_blocks Greg Kroah-Hartman
     [not found] ` <20170605153021.008595891@linuxfoundation.org>
2017-06-05 19:13   ` [PATCH 3.18 01/33] Revert "stackprotector: Increase the per-task stack canarys random range from 32 bits to 64 bits on 64-bit platforms" Kees Cook
2017-06-05 20:06     ` [kernel-hardening] " Eric Biggers
2017-06-06  7:21       ` Greg Kroah-Hartman
2017-06-05 20:34 ` [PATCH 3.18 00/33] 3.18.56-stable review Shuah Khan
2017-06-05 22:04 ` Guenter Roeck
     [not found] ` <5935c692.0bd51c0a.350e1.3395@mx.google.com>
2017-06-06  7:22   ` Greg Kroah-Hartman
2017-06-06 20:55     ` Kevin Hilman

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=20170605153021.550152891@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=vegard.nossum@oracle.com \
    --cc=weiwan@google.com \
    /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;
as well as URLs for NNTP newsgroup(s).