From patchwork Sun May 29 20:33:39 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sven Eckelmann X-Patchwork-Id: 16312 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 8454581839; Sun, 29 May 2016 22:33:51 +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=QjVoX5Ad; 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 C0E2A8054E for ; Sun, 29 May 2016 22:33:49 +0200 (CEST) Received: from sven-desktop.home.narfation.org (p200300C593C609FD0000000000002E16.dip0.t-ipconnect.de [IPv6:2003:c5:93c6:9fd::2e16]) by v3-1039.vlinux.de (Postfix) with ESMTPSA id 40923110119 for ; Sun, 29 May 2016 22:33:49 +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=1464554029; bh=FXpMsTJtEWBVmoEjtmHD1F3WC9aFW9/cLXqOUWKfs4M=; h=From:To:Subject:Date:From; b=QjVoX5Adtn9TY/V03WYYeQlnWhTSHNxb7IK6LdYMHxjbDz9E3Z5t2Uz/YVQBEOfw0 Epv3is3s5fp2qv0fmpKfp04HUnZi53+nxmQC3rJbo+iF8Ab7qmJv4bMy2RGPDDUlr7 7cWF1ChyqlKSmPJz0DfuesTzCCnEupuwGkzbrfHI= From: Sven Eckelmann To: b.a.t.m.a.n@lists.open-mesh.org Date: Sun, 29 May 2016 22:33:39 +0200 Message-Id: <1464553842-4680-1-git-send-email-sven@narfation.org> X-Mailer: git-send-email 2.8.1 Subject: [B.A.T.M.A.N.] [PATCH maint] batman-adv: Fix use-after-free of tt_req_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 tt_req_node is added and removed from a list inside a spinlock. But the locking is sometimes removed even when the object is still referenced and will be used later via this reference. For example batadv_send_tt_request can create a new tt_req_node (including add to a list) and later re-acquires the lock to remove it from the list and to free it. But at this time another context could have already removed this tt_req_node from the list and freed it. This can only be solved via reference counting to allow multiple contexts to handle the list manipulation while making sure that only the last context frees the list. Fixes: cea194d90b11 ("batman-adv: improved client announcement mechanism") Signed-off-by: Sven Eckelmann --- Cc: Matthias Schiffer : this can also be interesting for batman-adv-legacy installations on servers with more than one core net/batman-adv/translation-table.c | 27 +++++++++++++++++++++++---- net/batman-adv/types.h | 2 ++ 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c index 48adb91..46f90c5 100644 --- a/net/batman-adv/translation-table.c +++ b/net/batman-adv/translation-table.c @@ -2272,6 +2272,19 @@ static u32 batadv_tt_local_crc(struct batadv_priv *bat_priv, return crc; } +/** + * batadv_tt_req_node_release - free tt_req node entry + * @ref: kref pointer of the tt req_node entry + */ +static void batadv_tt_req_node_release(struct kref *ref) +{ + struct batadv_tt_req_node *node; + + node = container_of(ref, struct batadv_tt_req_node, refcount); + + kfree(node); +} + static void batadv_tt_req_list_free(struct batadv_priv *bat_priv) { struct batadv_tt_req_node *node; @@ -2281,7 +2294,7 @@ static void batadv_tt_req_list_free(struct batadv_priv *bat_priv) hlist_for_each_entry_safe(node, safe, &bat_priv->tt.req_list, list) { hlist_del_init(&node->list); - kfree(node); + kref_put(&node->refcount, batadv_tt_req_node_release); } spin_unlock_bh(&bat_priv->tt.req_list_lock); @@ -2318,7 +2331,7 @@ static void batadv_tt_req_purge(struct batadv_priv *bat_priv) if (batadv_has_timed_out(node->issued_at, BATADV_TT_REQUEST_TIMEOUT)) { hlist_del_init(&node->list); - kfree(node); + kref_put(&node->refcount, batadv_tt_req_node_release); } } spin_unlock_bh(&bat_priv->tt.req_list_lock); @@ -2350,9 +2363,11 @@ batadv_tt_req_node_new(struct batadv_priv *bat_priv, if (!tt_req_node) goto unlock; + kref_init(&tt_req_node->refcount); ether_addr_copy(tt_req_node->addr, orig_node->orig); tt_req_node->issued_at = jiffies; + kref_get(&tt_req_node->refcount); hlist_add_head(&tt_req_node->list, &bat_priv->tt.req_list); unlock: spin_unlock_bh(&bat_priv->tt.req_list_lock); @@ -2619,9 +2634,13 @@ out: spin_lock_bh(&bat_priv->tt.req_list_lock); /* hlist_del_init() verifies tt_req_node still is in the list */ hlist_del_init(&tt_req_node->list); + kref_put(&tt_req_node->refcount, batadv_tt_req_node_release); spin_unlock_bh(&bat_priv->tt.req_list_lock); - kfree(tt_req_node); } + + if (tt_req_node) + kref_put(&tt_req_node->refcount, batadv_tt_req_node_release); + kfree(tvlv_tt_data); return ret; } @@ -3057,7 +3076,7 @@ static void batadv_handle_tt_response(struct batadv_priv *bat_priv, if (!batadv_compare_eth(node->addr, resp_src)) continue; hlist_del_init(&node->list); - kfree(node); + kref_put(&node->refcount, batadv_tt_req_node_release); } spin_unlock_bh(&bat_priv->tt.req_list_lock); diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h index 1e47fbe..d75beef 100644 --- a/net/batman-adv/types.h +++ b/net/batman-adv/types.h @@ -1129,11 +1129,13 @@ struct batadv_tt_change_node { * struct batadv_tt_req_node - data to keep track of the tt requests in flight * @addr: mac address address of the originator this request was sent to * @issued_at: timestamp used for purging stale tt requests + * @refcount: number of contexts the object is used by * @list: list node for batadv_priv_tt::req_list */ struct batadv_tt_req_node { u8 addr[ETH_ALEN]; unsigned long issued_at; + struct kref refcount; struct hlist_node list; };