From patchwork Mon Jun 26 15:47:15 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Amit Pundir X-Patchwork-Id: 106349 Delivered-To: patch@linaro.org Received: by 10.140.101.48 with SMTP id t45csp161286qge; Mon, 26 Jun 2017 08:47:41 -0700 (PDT) X-Received: by 10.98.135.140 with SMTP id i134mr725492pfe.237.1498492060926; Mon, 26 Jun 2017 08:47:40 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1498492060; cv=none; d=google.com; s=arc-20160816; b=K57xg75x/qtcIcQaaA7nYEWyIvpV++Z43vjH+b853nHZgIut2VRaLGiMNQ9mGVIayC PX27FZA09W1NKg/FwNBNauzOe8Ab6UP/0PK8gV3kmhILL9PIkK8Zkjm+oBSxbRMJDUyY TmCIgA9shDm9umqv1r/uuONFcD8IGE+y6h9Yp8cKk+UKkhOGJnjLthR5bi/KZAmJn0ST imNaW7YhwnQHNlCVfob2KnUgqz8h0GiThAfN9glsb++VkbBF14feqX7IQbIImMn8l2t/ kMPxC3kG+9qUnWAsumDRFMScDEdlaksBRBSGDzlUxzCwa2fbf48X4052kL53j+PHjGx8 Psdg== 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=qpJCWHaNGV/2c0uUgAschRusRLR7ElygXndz0L0fknY=; b=C6blUAo4VduIsFjk0xEpcmRJyzAh05USQAwD5l9v9GqosQmc8bAS/VR23pZMuoDwth fuT2YIgg2xHtsgJisyOey0j1TsYw/6UbxHgdChsZ9px6+6cKU5/NOQ9dl5WaIU122KOb vVX6Pn8MUcBqLuVTACs7cT3b3BAdu0IsWdG/0HcZmE+lMXbdalvExNmjmh5IWmodEmaT 75rKP2UE7AToZfLHqvlLydGbXIquIbhAB9xdw71PjneCwWFjdqhw78cMN4GlSfKOHFGO kGgC5sBSO3JDZTYr8WwKdlLxkET2+MPjGhKENWAPmo3wPwR4uzZhyv8Soa7p1pJizfu/ +R6Q== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.b=cdUSz1Ii; 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 63si282868plf.50.2017.06.26.08.47.40; Mon, 26 Jun 2017 08:47:40 -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=cdUSz1Ii; 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 S1751960AbdFZPr3 (ORCPT + 6 others); Mon, 26 Jun 2017 11:47:29 -0400 Received: from mail-pf0-f173.google.com ([209.85.192.173]:36624 "EHLO mail-pf0-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751940AbdFZPr3 (ORCPT ); Mon, 26 Jun 2017 11:47:29 -0400 Received: by mail-pf0-f173.google.com with SMTP id q86so2214135pfl.3 for ; Mon, 26 Jun 2017 08:47:29 -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=qpJCWHaNGV/2c0uUgAschRusRLR7ElygXndz0L0fknY=; b=cdUSz1IinvHDdPSO+IKKBqZVM7E1++ozd1gAynUQ38SDVoPPNeP8Vl2555QnWCUbHp D0qk8SNpvAFmNxwm3LF3S0OieHZZfTRig66a1E6G7PbCpAdMuzw7bX6xF9aiy3FhBb0r 7GAz0Nfw5meIUo1oYhbtLxGWyzL0Xn6yMdxAs= 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=qpJCWHaNGV/2c0uUgAschRusRLR7ElygXndz0L0fknY=; b=UpGSVqI+1BHekXDr8aH1X2UAA/t0he9LmBmfRxUnUzAcpbTmJr4AhbXRJL5yztXmuM ypq6/+1SEqscYTjxwaQt4pgF2BD+5poyInd2ul1TA4/xfROnRui+nXKAiigFKOtLGJTF buva+Mbi+oon7OHKfU3VYTjzD4LmThDn/wT3gGk4mru2MTHzgxi8/md9sjw2YsKLTMiw 6w6mcUMnF4ntJ2Qe1IGqJ40HtxwqZG+51C/mZBD4/tJHGjWIPQMzD3/LayIhl2Kx7TmD WJKKiPwyIpx4hff6nVZbSKphVoS2eidROGAKgd6k80Ev1Wew1OYraD4PgOn/kOLvLCcJ Q71g== X-Gm-Message-State: AKS2vOziT6pweciq/845iluBb3indtdR96YQesqzgFLyonDwj2xpGkMO 27h74vckAVqfhmky X-Received: by 10.101.75.199 with SMTP id p7mr776737pgr.164.1498492048292; Mon, 26 Jun 2017 08:47:28 -0700 (PDT) Received: from localhost.localdomain ([106.51.139.251]) by smtp.gmail.com with ESMTPSA id s9sm829854pfe.21.2017.06.26.08.47.25 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 26 Jun 2017 08:47:27 -0700 (PDT) From: Amit Pundir To: Greg KH , Guillaume Nault , "David S . Miller" Cc: Stable Subject: [PATCH for-4.9 1/5] l2tp: fix race in l2tp_recv_common() Date: Mon, 26 Jun 2017 21:17:15 +0530 Message-Id: <1498492039-26905-2-git-send-email-amit.pundir@linaro.org> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1498492039-26905-1-git-send-email-amit.pundir@linaro.org> References: <1498492039-26905-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: Guillaume Nault commit 61b9a047729bb230978178bca6729689d0c50ca2 upstream. Taking a reference on sessions in l2tp_recv_common() is racy; this has to be done by the callers. To this end, a new function is required (l2tp_session_get()) to atomically lookup a session and take a reference on it. Callers then have to manually drop this reference. Fixes: fd558d186df2 ("l2tp: Split pppol2tp patch into separate l2tp and ppp parts") Signed-off-by: Guillaume Nault Signed-off-by: David S. Miller Signed-off-by: Amit Pundir --- net/l2tp/l2tp_core.c | 73 ++++++++++++++++++++++++++++++++++++++++++---------- net/l2tp/l2tp_core.h | 3 +++ net/l2tp/l2tp_ip.c | 17 ++++++++---- net/l2tp/l2tp_ip6.c | 18 +++++++++---- 4 files changed, 88 insertions(+), 23 deletions(-) -- 2.7.4 diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index e702cb95b89b..046a5ba0ebd2 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -278,6 +278,55 @@ struct l2tp_session *l2tp_session_find(struct net *net, struct l2tp_tunnel *tunn } EXPORT_SYMBOL_GPL(l2tp_session_find); +/* Like l2tp_session_find() but takes a reference on the returned session. + * Optionally calls session->ref() too if do_ref is true. + */ +struct l2tp_session *l2tp_session_get(struct net *net, + struct l2tp_tunnel *tunnel, + u32 session_id, bool do_ref) +{ + struct hlist_head *session_list; + struct l2tp_session *session; + + if (!tunnel) { + struct l2tp_net *pn = l2tp_pernet(net); + + session_list = l2tp_session_id_hash_2(pn, session_id); + + rcu_read_lock_bh(); + hlist_for_each_entry_rcu(session, session_list, global_hlist) { + if (session->session_id == session_id) { + l2tp_session_inc_refcount(session); + if (do_ref && session->ref) + session->ref(session); + rcu_read_unlock_bh(); + + return session; + } + } + rcu_read_unlock_bh(); + + return NULL; + } + + session_list = l2tp_session_id_hash(tunnel, session_id); + read_lock_bh(&tunnel->hlist_lock); + hlist_for_each_entry(session, session_list, hlist) { + if (session->session_id == session_id) { + l2tp_session_inc_refcount(session); + if (do_ref && session->ref) + session->ref(session); + read_unlock_bh(&tunnel->hlist_lock); + + return session; + } + } + read_unlock_bh(&tunnel->hlist_lock); + + return NULL; +} +EXPORT_SYMBOL_GPL(l2tp_session_get); + struct l2tp_session *l2tp_session_get_nth(struct l2tp_tunnel *tunnel, int nth, bool do_ref) { @@ -637,6 +686,9 @@ static int l2tp_recv_data_seq(struct l2tp_session *session, struct sk_buff *skb) * a data (not control) frame before coming here. Fields up to the * session-id have already been parsed and ptr points to the data * after the session-id. + * + * session->ref() must have been called prior to l2tp_recv_common(). + * session->deref() will be called automatically after skb is processed. */ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb, unsigned char *ptr, unsigned char *optr, u16 hdrflags, @@ -646,14 +698,6 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb, int offset; u32 ns, nr; - /* The ref count is increased since we now hold a pointer to - * the session. Take care to decrement the refcnt when exiting - * this function from now on... - */ - l2tp_session_inc_refcount(session); - if (session->ref) - (*session->ref)(session); - /* Parse and check optional cookie */ if (session->peer_cookie_len > 0) { if (memcmp(ptr, &session->peer_cookie[0], session->peer_cookie_len)) { @@ -806,8 +850,6 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb, /* Try to dequeue as many skbs from reorder_q as we can. */ l2tp_recv_dequeue(session); - l2tp_session_dec_refcount(session); - return; discard: @@ -816,8 +858,6 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb, if (session->deref) (*session->deref)(session); - - l2tp_session_dec_refcount(session); } EXPORT_SYMBOL(l2tp_recv_common); @@ -924,8 +964,14 @@ static int l2tp_udp_recv_core(struct l2tp_tunnel *tunnel, struct sk_buff *skb, } /* Find the session context */ - session = l2tp_session_find(tunnel->l2tp_net, tunnel, session_id); + session = l2tp_session_get(tunnel->l2tp_net, tunnel, session_id, true); if (!session || !session->recv_skb) { + if (session) { + if (session->deref) + session->deref(session); + l2tp_session_dec_refcount(session); + } + /* Not found? Pass to userspace to deal with */ l2tp_info(tunnel, L2TP_MSG_DATA, "%s: no session found (%u/%u). Passing up.\n", @@ -934,6 +980,7 @@ static int l2tp_udp_recv_core(struct l2tp_tunnel *tunnel, struct sk_buff *skb, } l2tp_recv_common(session, skb, ptr, optr, hdrflags, length, payload_hook); + l2tp_session_dec_refcount(session); return 0; diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h index e7233bad65e0..1d020505bf06 100644 --- a/net/l2tp/l2tp_core.h +++ b/net/l2tp/l2tp_core.h @@ -240,6 +240,9 @@ static inline struct l2tp_tunnel *l2tp_sock_to_tunnel(struct sock *sk) return tunnel; } +struct l2tp_session *l2tp_session_get(struct net *net, + struct l2tp_tunnel *tunnel, + u32 session_id, bool do_ref); struct l2tp_session *l2tp_session_find(struct net *net, struct l2tp_tunnel *tunnel, u32 session_id); diff --git a/net/l2tp/l2tp_ip.c b/net/l2tp/l2tp_ip.c index 20669537816e..3468d5635d0a 100644 --- a/net/l2tp/l2tp_ip.c +++ b/net/l2tp/l2tp_ip.c @@ -143,19 +143,19 @@ static int l2tp_ip_recv(struct sk_buff *skb) } /* Ok, this is a data packet. Lookup the session. */ - session = l2tp_session_find(net, NULL, session_id); - if (session == NULL) + session = l2tp_session_get(net, NULL, session_id, true); + if (!session) goto discard; tunnel = session->tunnel; - if (tunnel == NULL) - goto discard; + if (!tunnel) + goto discard_sess; /* Trace packet contents, if enabled */ if (tunnel->debug & L2TP_MSG_DATA) { length = min(32u, skb->len); if (!pskb_may_pull(skb, length)) - goto discard; + goto discard_sess; /* Point to L2TP header */ optr = ptr = skb->data; @@ -165,6 +165,7 @@ static int l2tp_ip_recv(struct sk_buff *skb) } l2tp_recv_common(session, skb, ptr, optr, 0, skb->len, tunnel->recv_payload_hook); + l2tp_session_dec_refcount(session); return 0; @@ -203,6 +204,12 @@ static int l2tp_ip_recv(struct sk_buff *skb) return sk_receive_skb(sk, skb, 1); +discard_sess: + if (session->deref) + session->deref(session); + l2tp_session_dec_refcount(session); + goto discard; + discard_put: sock_put(sk); diff --git a/net/l2tp/l2tp_ip6.c b/net/l2tp/l2tp_ip6.c index a4b0c9232bf1..b10abef6b0a0 100644 --- a/net/l2tp/l2tp_ip6.c +++ b/net/l2tp/l2tp_ip6.c @@ -156,19 +156,19 @@ static int l2tp_ip6_recv(struct sk_buff *skb) } /* Ok, this is a data packet. Lookup the session. */ - session = l2tp_session_find(net, NULL, session_id); - if (session == NULL) + session = l2tp_session_get(net, NULL, session_id, true); + if (!session) goto discard; tunnel = session->tunnel; - if (tunnel == NULL) - goto discard; + if (!tunnel) + goto discard_sess; /* Trace packet contents, if enabled */ if (tunnel->debug & L2TP_MSG_DATA) { length = min(32u, skb->len); if (!pskb_may_pull(skb, length)) - goto discard; + goto discard_sess; /* Point to L2TP header */ optr = ptr = skb->data; @@ -179,6 +179,8 @@ static int l2tp_ip6_recv(struct sk_buff *skb) l2tp_recv_common(session, skb, ptr, optr, 0, skb->len, tunnel->recv_payload_hook); + l2tp_session_dec_refcount(session); + return 0; pass_up: @@ -216,6 +218,12 @@ static int l2tp_ip6_recv(struct sk_buff *skb) return sk_receive_skb(sk, skb, 1); +discard_sess: + if (session->deref) + session->deref(session); + l2tp_session_dec_refcount(session); + goto discard; + discard_put: sock_put(sk);