From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1CAADC4332B for ; Thu, 19 Mar 2020 13:07:12 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id DD9E820836 for ; Thu, 19 Mar 2020 13:07:11 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1584623232; bh=5GdR2swVliXuxsOsPP7mk6l4w61TEMDgCeFcDYqZRx0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=AJrgiawpQceIzAa0j/a8WzNqVqq1YFLBzsGNYVFbXhUH2jyytOj/uWFBOjWEYI5mx uwbl/McLXT6C3By1AVBSXHMxp1Mw7BTtHwRXnPlbES3/H8eV65LMDnzMPg7RIn5vnq hSYVmNLqYavc0FWZ+q8ACQWCLCFT0kuJyWQxyixM= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727854AbgCSNHK (ORCPT ); Thu, 19 Mar 2020 09:07:10 -0400 Received: from mail.kernel.org ([198.145.29.99]:50592 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727822AbgCSNHF (ORCPT ); Thu, 19 Mar 2020 09:07:05 -0400 Received: from localhost (83-86-89-107.cable.dynamic.v4.ziggo.nl [83.86.89.107]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 1F5A3207FC; Thu, 19 Mar 2020 13:07:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1584623224; bh=5GdR2swVliXuxsOsPP7mk6l4w61TEMDgCeFcDYqZRx0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=S/wZ4H2xSMngR7OcPHfjLtmIyTa5z5p9+Mofm1EiduS+bTru4OYV4cBicHuiRInme WJu0prS7oANOfwdUAVaOx5LyKeRLWg3YmIcq/TQvjy8iEXUGYsrHX+8OwsC7pXPyJp iym4K4Cm2z5p5eTw0hzH2Db4eLag2siTd/cidF70= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org, stable@vger.kernel.org Cc: Greg Kroah-Hartman , Sven Eckelmann , Martin Weinelt , Amadeus Alfa , Marek Lindner , "David S. Miller" Subject: [PATCH 4.4 50/93] batman-adv: Fix use-after-free/double-free of tt_req_node Date: Thu, 19 Mar 2020 13:59:54 +0100 Message-Id: <20200319123940.847996821@linuxfoundation.org> X-Mailer: git-send-email 2.25.2 In-Reply-To: <20200319123924.795019515@linuxfoundation.org> References: <20200319123924.795019515@linuxfoundation.org> User-Agent: quilt/0.66 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Sven Eckelmann commit 9c4604a298e0a9807eaf2cd912d1ebf24d98fbeb upstream. 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. CPU#0 batadv_batman_skb_recv from net_device 0 -> batadv_iv_ogm_receive -> batadv_iv_ogm_process -> batadv_iv_ogm_process_per_outif -> batadv_tvlv_ogm_receive -> batadv_tvlv_ogm_receive -> batadv_tvlv_containers_process -> batadv_tvlv_call_handler -> batadv_tt_tvlv_ogm_handler_v1 -> batadv_tt_update_orig -> batadv_send_tt_request -> batadv_tt_req_node_new spin_lock(...) allocates new tt_req_node and adds it to list spin_unlock(...) return tt_req_node CPU#1 batadv_batman_skb_recv from net_device 1 -> batadv_recv_unicast_tvlv -> batadv_tvlv_containers_process -> batadv_tvlv_call_handler -> batadv_tt_tvlv_unicast_handler_v1 -> batadv_handle_tt_response spin_lock(...) tt_req_node gets removed from list and is freed spin_unlock(...) CPU#0 <- returned to batadv_send_tt_request spin_lock(...) tt_req_node gets removed from list and is freed MEMORY CORRUPTION/SEGFAULT/... spin_unlock(...) 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 holding a reference will free the object. Fixes: a73105b8d4c7 ("batman-adv: improved client announcement mechanism") Signed-off-by: Sven Eckelmann Tested-by: Martin Weinelt Tested-by: Amadeus Alfa Signed-off-by: Marek Lindner Signed-off-by: David S. Miller Signed-off-by: Greg Kroah-Hartman --- net/batman-adv/translation-table.c | 43 +++++++++++++++++++++++++++++++------ net/batman-adv/types.h | 2 + 2 files changed, 39 insertions(+), 6 deletions(-) --- a/net/batman-adv/translation-table.c +++ b/net/batman-adv/translation-table.c @@ -2206,6 +2206,29 @@ static u32 batadv_tt_local_crc(struct ba 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 *tt_req_node; + + tt_req_node = container_of(ref, struct batadv_tt_req_node, refcount); + + kfree(tt_req_node); +} + +/** + * batadv_tt_req_node_put - decrement the tt_req_node refcounter and + * possibly release it + * @tt_req_node: tt_req_node to be free'd + */ +static void batadv_tt_req_node_put(struct batadv_tt_req_node *tt_req_node) +{ + kref_put(&tt_req_node->refcount, batadv_tt_req_node_release); +} + static void batadv_tt_req_list_free(struct batadv_priv *bat_priv) { struct batadv_tt_req_node *node; @@ -2215,7 +2238,7 @@ static void batadv_tt_req_list_free(stru hlist_for_each_entry_safe(node, safe, &bat_priv->tt.req_list, list) { hlist_del_init(&node->list); - kfree(node); + batadv_tt_req_node_put(node); } spin_unlock_bh(&bat_priv->tt.req_list_lock); @@ -2252,7 +2275,7 @@ static void batadv_tt_req_purge(struct b if (batadv_has_timed_out(node->issued_at, BATADV_TT_REQUEST_TIMEOUT)) { hlist_del_init(&node->list); - kfree(node); + batadv_tt_req_node_put(node); } } spin_unlock_bh(&bat_priv->tt.req_list_lock); @@ -2284,9 +2307,11 @@ batadv_tt_req_node_new(struct batadv_pri 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); @@ -2536,13 +2561,19 @@ static int batadv_send_tt_request(struct out: if (primary_if) batadv_hardif_free_ref(primary_if); + if (ret && tt_req_node) { 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); + if (!hlist_unhashed(&tt_req_node->list)) { + hlist_del_init(&tt_req_node->list); + batadv_tt_req_node_put(tt_req_node); + } spin_unlock_bh(&bat_priv->tt.req_list_lock); - kfree(tt_req_node); } + + if (tt_req_node) + batadv_tt_req_node_put(tt_req_node); + kfree(tvlv_tt_data); return ret; } @@ -2978,7 +3009,7 @@ static void batadv_handle_tt_response(st if (!batadv_compare_eth(node->addr, resp_src)) continue; hlist_del_init(&node->list); - kfree(node); + batadv_tt_req_node_put(node); } spin_unlock_bh(&bat_priv->tt.req_list_lock); --- a/net/batman-adv/types.h +++ b/net/batman-adv/types.h @@ -1001,11 +1001,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; };