From patchwork Tue Mar 19 15:05:44 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Guillaume Nault X-Patchwork-Id: 1058459 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 44NxFW6db5z9s5c for ; Wed, 20 Mar 2019 02:05:51 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727487AbfCSPFt (ORCPT ); Tue, 19 Mar 2019 11:05:49 -0400 Received: from mail-wr1-f68.google.com ([209.85.221.68]:39588 "EHLO mail-wr1-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726703AbfCSPFt (ORCPT ); Tue, 19 Mar 2019 11:05:49 -0400 Received: by mail-wr1-f68.google.com with SMTP id j9so7404622wrn.6 for ; Tue, 19 Mar 2019 08:05:48 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:mime-version :content-disposition:user-agent; bh=NCVir6UCMoh3BAX4tdN8zvcQWCnTwvrr+MEGpDdcxY4=; b=gjOk1bKw6s3iGFu3xwt1GIML61k/UjUAGRd8Qa/ruop0AK/naxpZHKYtJMBjkrhAwQ 7tFihO8ZoopiEEib3LZ0dA4VpD0xeNockgJ08R3P/xurn9rFRWIOnbwpdFtOzZDogqU5 Uwu3uIDDn4Mxr6+/7gOKcIK6erfZ8NKR6AwM9wM+6eI6dKwMdxi8cC2tq/8VgGJHgEqk yxsMvMD/r9CU8AB/5X+ZMEqmqorGqF5bHwDRoQE5EgXlnUcD2YFr3i9FHZ7lpFg1ieDD doFAODCO2iBvvB/aQOi4yfC2WlCPwvyuCMG9xqVAKcVRuiQyZ69e3KO8MHH/US4fHE7c /V8Q== X-Gm-Message-State: APjAAAUGdPS0ByoGhRzFhpRm/6fYFjbgNVZ0SdBrKqk5dRETAc+IGitL L3ky6vzhk3Je8sMEyf3cK8qUr6VUJFc= X-Google-Smtp-Source: APXvYqzsRI2k7TZ5h3GyAEWGZt/xxrlznj/CWcLqwxMDKO0DNXm6/9YEjpE4l0+nl3ntSxcIuDvMnQ== X-Received: by 2002:adf:eecb:: with SMTP id a11mr10313169wrp.23.1553007947469; Tue, 19 Mar 2019 08:05:47 -0700 (PDT) Received: from pc-2.home (2a01cb05850ddf00045dd60e6368f84b.ipv6.abo.wanadoo.fr. [2a01:cb05:850d:df00:45d:d60e:6368:f84b]) by smtp.gmail.com with ESMTPSA id a9sm2592580wmm.10.2019.03.19.08.05.46 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 19 Mar 2019 08:05:46 -0700 (PDT) Date: Tue, 19 Mar 2019 16:05:44 +0100 From: Guillaume Nault To: netdev@vger.kernel.org Cc: Eric Dumazet Subject: [PATCH net-next] tcp: free request sock directly upon TFO or syncookies error Message-ID: MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.10.1 (2018-07-13) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Since the request socket is created locally, it'd make more sense to use reqsk_free() instead of reqsk_put() in TFO and syncookies' error path. However, tcp_get_cookie_sock() may set ->rsk_refcnt before freeing the socket; tcp_conn_request() may also have non-null ->rsk_refcnt because of tcp_try_fastopen(). In both cases 'req' hasn't been exposed to the outside world and is safe to free immediately, but that'd trigger the WARN_ON_ONCE in reqsk_free(). Define __reqsk_free() for these situations where we know nobody's referencing the socket, even though ->rsk_refcnt might be non-null. Now we can consolidate the error path of tcp_get_cookie_sock() and tcp_conn_request(). Signed-off-by: Guillaume Nault Signed-off-by: Eric Dumazet --- include/net/request_sock.h | 10 +++++++--- net/ipv4/syncookies.c | 17 ++++++++--------- net/ipv4/tcp_input.c | 5 ++--- 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/include/net/request_sock.h b/include/net/request_sock.h index 21a5243fecd1..9dfd7960d90a 100644 --- a/include/net/request_sock.h +++ b/include/net/request_sock.h @@ -106,10 +106,8 @@ reqsk_alloc(const struct request_sock_ops *ops, struct sock *sk_listener, return req; } -static inline void reqsk_free(struct request_sock *req) +static inline void __reqsk_free(struct request_sock *req) { - WARN_ON_ONCE(refcount_read(&req->rsk_refcnt) != 0); - req->rsk_ops->destructor(req); if (req->rsk_listener) sock_put(req->rsk_listener); @@ -117,6 +115,12 @@ static inline void reqsk_free(struct request_sock *req) kmem_cache_free(req->rsk_ops->slab, req); } +static inline void reqsk_free(struct request_sock *req) +{ + WARN_ON_ONCE(refcount_read(&req->rsk_refcnt) != 0); + __reqsk_free(req); +} + static inline void reqsk_put(struct request_sock *req) { if (refcount_dec_and_test(&req->rsk_refcnt)) diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c index e531344611a0..008545f63667 100644 --- a/net/ipv4/syncookies.c +++ b/net/ipv4/syncookies.c @@ -216,16 +216,15 @@ struct sock *tcp_get_cookie_sock(struct sock *sk, struct sk_buff *skb, refcount_set(&req->rsk_refcnt, 1); tcp_sk(child)->tsoffset = tsoff; sock_rps_save_rxhash(child, skb); - if (!inet_csk_reqsk_queue_add(sk, req, child)) { - bh_unlock_sock(child); - sock_put(child); - child = NULL; - reqsk_put(req); - } - } else { - reqsk_free(req); + if (inet_csk_reqsk_queue_add(sk, req, child)) + return child; + + bh_unlock_sock(child); + sock_put(child); } - return child; + __reqsk_free(req); + + return NULL; } EXPORT_SYMBOL(tcp_get_cookie_sock); diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c index 5def3c48870e..5dfbc333e79a 100644 --- a/net/ipv4/tcp_input.c +++ b/net/ipv4/tcp_input.c @@ -6502,8 +6502,7 @@ int tcp_conn_request(struct request_sock_ops *rsk_ops, reqsk_fastopen_remove(fastopen_sk, req, false); bh_unlock_sock(fastopen_sk); sock_put(fastopen_sk); - reqsk_put(req); - goto drop; + goto drop_and_free; } sk->sk_data_ready(sk); bh_unlock_sock(fastopen_sk); @@ -6527,7 +6526,7 @@ int tcp_conn_request(struct request_sock_ops *rsk_ops, drop_and_release: dst_release(dst); drop_and_free: - reqsk_free(req); + __reqsk_free(req); drop: tcp_listendrop(sk); return 0;