From patchwork Sun Mar 6 10:07:22 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sven Eckelmann X-Patchwork-Id: 15901 Return-Path: X-Original-To: patchwork@open-mesh.org Delivered-To: patchwork@open-mesh.org Received: from open-mesh.org (localhost [127.0.0.1]) by open-mesh.org (Postfix) with ESMTP id 631A28240A; Sun, 6 Mar 2016 11:07:38 +0100 (CET) Authentication-Results: open-mesh.org; dkim=fail reason="verification failed; unprotected key" header.d=narfation.org header.i=@narfation.org header.b=LhLCmYzs; dkim-adsp=fail (unprotected policy); dkim-atps=neutral Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=79.140.41.39; 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 [79.140.41.39]) by open-mesh.org (Postfix) with ESMTPS id BA158823FC for ; Sun, 6 Mar 2016 11:07:36 +0100 (CET) Received: from sven-desktop.home.narfation.org (p200300C593C41CFD0000000000002E16.dip0.t-ipconnect.de [IPv6:2003:c5:93c4:1cfd::2e16]) by v3-1039.vlinux.de (Postfix) with ESMTPSA id 10424110103; Sun, 6 Mar 2016 11:07:36 +0100 (CET) Authentication-Results: v3-1039.vlinux.de; dmarc=none header.from=narfation.org DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=narfation.org; s=20121; t=1457258856; bh=HO0MqnbLrHWlzwcXsXXW+oBXPp5t9lR+4gkD6Q1pqq0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=LhLCmYzsZx8kI25qTdyyKvxAzm0qzwdP9npwXa9Lagkot262Pfwn1PvlxyZ7R5BYt CxGdodep2VqS7y6e70aEQZNd9snZ9PpLUyY5OE1zW8j3tnpdauQ3/RsLIumwB3TKSi aY1AQtdanS/x62eyXKVLAjFiD/a/DzsPPKykC8Ys= From: Sven Eckelmann To: b.a.t.m.a.n@lists.open-mesh.org Date: Sun, 6 Mar 2016 11:07:22 +0100 Message-Id: <1457258842-10389-2-git-send-email-sven@narfation.org> X-Mailer: git-send-email 2.7.0 In-Reply-To: <1457258842-10389-1-git-send-email-sven@narfation.org> References: <1457258842-10389-1-git-send-email-sven@narfation.org> Subject: [B.A.T.M.A.N.] [RFC maint 2/2] batman-adv: Fix reference counting of hardif_neigh_node object for neigh_node 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 batadv_neigh_node was specific to a batadv_hardif_neigh_node and held an implicit reference to it. But this reference was never stored in form of a pointer in the batadv_neigh_node itself. Instead batadv_neigh_node_release depends on a consistent state of hard_iface->neigh_list and that batadv_hardif_neigh_get always returns the batadv_hardif_neigh_node object which it has a reference for. But batadv_hardif_neigh_get cannot guarantee that because it is working only with rcu_read_lock on this list. It can therefore happen that a neigh_addr is in this list twice or that batadv_hardif_neigh_get cannot find the batadv_hardif_neigh_node for an neigh_addr due to some other list operations taking place at the same time. Instead add a batadv_hardif_neigh_node pointer directly in batadv_neigh_node which will be used for the reference counter decremented on release of batadv_neigh_node. Fixes: fed2826b490c ("batman-adv: add list of unique single hop neighbors per hard-interface") Signed-off-by: Sven Eckelmann --- See https://www.open-mesh.org/issues/242 net/batman-adv/originator.c | 16 ++++++++++++---- net/batman-adv/types.h | 2 ++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/net/batman-adv/originator.c b/net/batman-adv/originator.c index 2d288ea..2f754d7 100644 --- a/net/batman-adv/originator.c +++ b/net/batman-adv/originator.c @@ -250,7 +250,6 @@ static void batadv_neigh_node_release(struct kref *ref) { struct hlist_node *node_tmp; struct batadv_neigh_node *neigh_node; - struct batadv_hardif_neigh_node *hardif_neigh; struct batadv_neigh_ifinfo *neigh_ifinfo; struct batadv_algo_ops *bao; @@ -262,6 +261,10 @@ static void batadv_neigh_node_release(struct kref *ref) batadv_neigh_ifinfo_put(neigh_ifinfo); } +#if 0 + /* THIS IS BAD BECAUSE THIS MAY NOT BE THE REAL OBJECT THIS + * neigh_node HAS THE REFERENCE FOR + */ hardif_neigh = batadv_hardif_neigh_get(neigh_node->if_incoming, neigh_node->addr); if (hardif_neigh) { @@ -269,6 +272,10 @@ static void batadv_neigh_node_release(struct kref *ref) batadv_hardif_neigh_put(hardif_neigh); batadv_hardif_neigh_put(hardif_neigh); } +#endif + + if (neigh_node->hardif_neigh) + batadv_hardif_neigh_put(neigh_node->hardif_neigh); if (bao->bat_neigh_free) bao->bat_neigh_free(neigh_node); @@ -648,6 +655,10 @@ batadv_neigh_node_new(struct batadv_orig_node *orig_node, neigh_node->if_incoming = hard_iface; neigh_node->orig_node = orig_node; + /* increment unique neighbor refcount */ + kref_get(&hardif_neigh->refcount); + neigh_node->hardif_neigh = hardif_neigh; + /* extra reference for return */ kref_init(&neigh_node->refcount); kref_get(&neigh_node->refcount); @@ -656,9 +667,6 @@ batadv_neigh_node_new(struct batadv_orig_node *orig_node, hlist_add_head_rcu(&neigh_node->list, &orig_node->neigh_list); spin_unlock_bh(&orig_node->neigh_list_lock); - /* increment unique neighbor refcount */ - kref_get(&hardif_neigh->refcount); - batadv_dbg(BATADV_DBG_BATMAN, orig_node->bat_priv, "Creating new neighbor %pM for orig_node %pM on interface %s\n", neigh_addr, orig_node->orig, hard_iface->net_dev->name); diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h index 1480538..58a3fa5 100644 --- a/net/batman-adv/types.h +++ b/net/batman-adv/types.h @@ -433,6 +433,7 @@ struct batadv_hardif_neigh_node { * @ifinfo_lock: lock protecting private ifinfo members and list * @if_incoming: pointer to incoming hard-interface * @last_seen: when last packet via this neighbor was received + * @hardif_neigh: hardif_neigh of this neighbor * @refcount: number of contexts the object is used * @rcu: struct used for freeing in an RCU-safe manner */ @@ -444,6 +445,7 @@ struct batadv_neigh_node { spinlock_t ifinfo_lock; /* protects ifinfo_list and its members */ struct batadv_hard_iface *if_incoming; unsigned long last_seen; + struct batadv_hardif_neigh_node *hardif_neigh; struct kref refcount; struct rcu_head rcu; };