From patchwork Thu Jun 30 18:11:34 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sven Eckelmann X-Patchwork-Id: 16410 X-Patchwork-Delegate: mareklindner@neomailbox.ch Return-Path: X-Original-To: patchwork@open-mesh.org Delivered-To: patchwork@open-mesh.org Received: from open-mesh.org (localhost [IPv6:::1]) by open-mesh.org (Postfix) with ESMTP id B3D2E81F8A; Thu, 30 Jun 2016 20:11:47 +0200 (CEST) Authentication-Results: open-mesh.org; dmarc=none header.from=narfation.org Authentication-Results: open-mesh.org; dkim=fail reason="verification failed; unprotected key" header.d=narfation.org header.i=@narfation.org header.b=K4ISQgOx; dkim-adsp=fail (unprotected policy); dkim-atps=neutral Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2001:4d88:2000:7::2; helo=v3-1039.vlinux.de; envelope-from=sven@narfation.org; receiver=b.a.t.m.a.n@lists.open-mesh.org Authentication-Results: open-mesh.org; dmarc=pass header.from=narfation.org Received: from v3-1039.vlinux.de (narfation.org [IPv6:2001:4d88:2000:7::2]) by open-mesh.org (Postfix) with ESMTPS id 9914E8224C for ; Thu, 30 Jun 2016 20:11:41 +0200 (CEST) Received: from sven-desktop.home.narfation.org (p200300C593C14EF96A1729FFFEE0E6EC.dip0.t-ipconnect.de [IPv6:2003:c5:93c1:4ef9:6a17:29ff:fee0:e6ec]) by v3-1039.vlinux.de (Postfix) with ESMTPSA id 064241100E8; Thu, 30 Jun 2016 20:11:40 +0200 (CEST) Authentication-Results: v3-1039.vlinux.de; dmarc=none header.from=narfation.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=narfation.org; s=20121; t=1467310301; bh=/w7509edu5r35aVWcfvf6s8LxR3GAwxIacpoRbnBFYg=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=K4ISQgOx3Y6r53T61B2utt1HsrQJvm76DvqSfIaoklahcHvOEUka6lY0HAI3S8kOn ctIDwDLa4/fmWy8iYG2F9jVSMXCDEGb7uHzUUZJfTwLQ/odC8o1gh/b+Msf8XpC2Tp koxh8rqNH/+wraeV0tqHyMQlwsf4m1PjvLxfE6Fo= From: Sven Eckelmann To: b.a.t.m.a.n@lists.open-mesh.org Date: Thu, 30 Jun 2016 20:11:34 +0200 Message-Id: <1467310294-5892-2-git-send-email-sven@narfation.org> X-Mailer: git-send-email 2.8.1 In-Reply-To: <1467310246-5820-1-git-send-email-sven@narfation.org> References: <1467310246-5820-1-git-send-email-sven@narfation.org> Subject: [B.A.T.M.A.N.] [PATCH v2 3/3] batman-adv: Fix reference leak in batadv_find_router X-BeenThere: b.a.t.m.a.n@lists.open-mesh.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: The list for a Better Approach To Mobile Ad-hoc Networking List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: The list for a Better Approach To Mobile Ad-hoc Networking Errors-To: b.a.t.m.a.n-bounces@lists.open-mesh.org Sender: "B.A.T.M.A.N" The replacement of last_bonding_candidate in batadv_orig_node has to be an atomic operation. Otherwise it is possible that the reference counter of a batadv_orig_ifinfo is reduced which was no longer the last_bonding_candidate when the new candidate is added. This can either lead to an invalid memory access or to reference leaks which make it impossible to an interface which was added to batman-adv. Fixes: 797edd9e87ac ("batman-adv: add bonding again") Signed-off-by: Sven Eckelmann Acked-by: Simon Wunderlich --- v2: - get refcnt for new selected router before assigning it to returned variable - move refcnt cleanup of all remembered candidates/routers to central place --- net/batman-adv/routing.c | 52 ++++++++++++++++++++++++++++++++++++------------ net/batman-adv/types.h | 4 +++- 2 files changed, 42 insertions(+), 14 deletions(-) diff --git a/net/batman-adv/routing.c b/net/batman-adv/routing.c index bba9276..a4253d4 100644 --- a/net/batman-adv/routing.c +++ b/net/batman-adv/routing.c @@ -462,6 +462,29 @@ static int batadv_check_unicast_packet(struct batadv_priv *bat_priv, } /** + * batadv_last_bonding_replace - Replace last_bonding_candidate of orig_node + * @orig_node: originator node whose bonding candidates should be replaced + * @new_candidate: new bonding candidate or NULL + */ +static void +batadv_last_bonding_replace(struct batadv_orig_node *orig_node, + struct batadv_orig_ifinfo *new_candidate) +{ + struct batadv_orig_ifinfo *old_candidate; + + spin_lock_bh(&orig_node->neigh_list_lock); + old_candidate = orig_node->last_bonding_candidate; + + if (new_candidate) + kref_get(&new_candidate->refcount); + orig_node->last_bonding_candidate = new_candidate; + spin_unlock_bh(&orig_node->neigh_list_lock); + + if (old_candidate) + batadv_orig_ifinfo_put(old_candidate); +} + +/** * batadv_find_router - find a suitable router for this originator * @bat_priv: the bat priv with all the soft interface information * @orig_node: the destination node @@ -568,10 +591,6 @@ next: } rcu_read_unlock(); - /* last_bonding_candidate is reset below, remove the old reference. */ - if (orig_node->last_bonding_candidate) - batadv_orig_ifinfo_put(orig_node->last_bonding_candidate); - /* After finding candidates, handle the three cases: * 1) there is a next candidate, use that * 2) there is no next candidate, use the first of the list @@ -580,21 +599,28 @@ next: if (next_candidate) { batadv_neigh_node_put(router); - /* remove references to first candidate, we don't need it. */ - if (first_candidate) { - batadv_neigh_node_put(first_candidate_router); - batadv_orig_ifinfo_put(first_candidate); - } + kref_get(&next_candidate_router->refcount); router = next_candidate_router; - orig_node->last_bonding_candidate = next_candidate; + batadv_last_bonding_replace(orig_node, next_candidate); } else if (first_candidate) { batadv_neigh_node_put(router); - /* refcounting has already been done in the loop above. */ + kref_get(&first_candidate_router->refcount); router = first_candidate_router; - orig_node->last_bonding_candidate = first_candidate; + batadv_last_bonding_replace(orig_node, first_candidate); } else { - orig_node->last_bonding_candidate = NULL; + batadv_last_bonding_replace(orig_node, NULL); + } + + /* cleanup of candidates */ + if (first_candidate) { + batadv_neigh_node_put(first_candidate_router); + batadv_orig_ifinfo_put(first_candidate); + } + + if (next_candidate) { + batadv_neigh_node_put(next_candidate_router); + batadv_orig_ifinfo_put(next_candidate); } return router; diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h index d82f6b4..96af6da 100644 --- a/net/batman-adv/types.h +++ b/net/batman-adv/types.h @@ -329,7 +329,9 @@ struct batadv_orig_node { DECLARE_BITMAP(bcast_bits, BATADV_TQ_LOCAL_WINDOW_SIZE); u32 last_bcast_seqno; struct hlist_head neigh_list; - /* neigh_list_lock protects: neigh_list and router */ + /* neigh_list_lock protects: neigh_list, ifinfo_list, + * last_bonding_candidate and router + */ spinlock_t neigh_list_lock; struct hlist_node hash_entry; struct batadv_priv *bat_priv;