From patchwork Tue Dec 18 19:40:02 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Frank Rowand X-Patchwork-Id: 1015584 Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 43K7yG4wFmz9s6w for ; Wed, 19 Dec 2018 06:54:14 +1100 (AEDT) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="oMesuXgO"; dkim-atps=neutral Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 43K7yG2wr0zDqZ2 for ; Wed, 19 Dec 2018 06:54:14 +1100 (AEDT) Authentication-Results: lists.ozlabs.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="oMesuXgO"; dkim-atps=neutral X-Original-To: linuxppc-dev@lists.ozlabs.org Delivered-To: linuxppc-dev@lists.ozlabs.org Authentication-Results: lists.ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gmail.com (client-ip=2607:f8b0:4864:20::443; helo=mail-pf1-x443.google.com; envelope-from=frowand.list@gmail.com; receiver=) Authentication-Results: lists.ozlabs.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="oMesuXgO"; dkim-atps=neutral Received: from mail-pf1-x443.google.com (mail-pf1-x443.google.com [IPv6:2607:f8b0:4864:20::443]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 43K7fL1MhNzDqPQ for ; Wed, 19 Dec 2018 06:40:25 +1100 (AEDT) Received: by mail-pf1-x443.google.com with SMTP id g62so8586170pfd.12 for ; Tue, 18 Dec 2018 11:40:25 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=FFIR56EPPMtxpRI59NyRK3PwR1fe0YI9MbaLl494NEI=; b=oMesuXgOxh3rffuZdb2VG3kMN2EgZm0eenELSQ9/8FZLHdrikxwu3pPoGFWqqSN/Ne O4AfSIWfCfUxfQAq6De1Shrzkh74aUdEqGlFNEExVweVkbp+12LftWeOKEsdorQI/tVI 94eMIt0dIyjhs4F4jIQ1GWGT55XeCupedkYFOLZTpFpvrXhq9G0Agj96LR62Wcx6fTFs xiVo2kTW1/ZgopJ1OfxUrkmmEThEud9IIp8KJjchWEJAV46INdfFZgqJhxJgMVn6qirT BPzoHYALpjGYQZXt4x17F1KQnaY8XXS2Vuen1KqfREZoPwTPsOArF7pecLnrJWctsPIl Oo9g== 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=FFIR56EPPMtxpRI59NyRK3PwR1fe0YI9MbaLl494NEI=; b=ro1fOGzxfr61M45GHmRLhDJslcC5NyGUrszHKdN7u2qj/W/C/ZklKsclyNRlQYNV6f AFml/MihLGOZrSzryMRH9HDRLle/P/zjzq+tzh21ZMOFz0iCKFZu4TjhOBIiXZaH9uiY QlYFoYiyrSIrgIXPHLcmzTKQKwnbGgtdBLKMXC16shxQh5lIIbW5f/XsRmEo9OTYz2Ja jUb+SiUAzlTKDS1V0anxb75JJFkZ2hBLjrC13UiboQzBv1c2EkarRBENe/M2fMNEuxYu SJonlm472I7dV1OPWivKaS0TjtG5dvgTgpPLgEoSivKZ72ch9nS9XGu4c1u2OES3k9x8 693w== X-Gm-Message-State: AA+aEWZ6cVunHiFHeamtmOii/bhkA10buXoeEdhEPPH089GyoqFoelTA +4ujIILIK30AK8JnNWdA1Gg= X-Google-Smtp-Source: AFSGD/XB5RxrA+Vo26Mis+VOsHyU9tLICH1XvHi0jMUo+CSL4elZ1+OfbPs5sTmWiZ1ddzmSKaS9ug== X-Received: by 2002:a62:de06:: with SMTP id h6mr18383172pfg.158.1545162024350; Tue, 18 Dec 2018 11:40:24 -0800 (PST) Received: from localhost.localdomain (c-24-6-192-50.hsd1.ca.comcast.net. [24.6.192.50]) by smtp.gmail.com with ESMTPSA id b202sm29493069pfb.88.2018.12.18.11.40.23 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Tue, 18 Dec 2018 11:40:23 -0800 (PST) From: frowand.list@gmail.com To: robh+dt@kernel.org, Michael Bringmann , linuxppc-dev@lists.ozlabs.org Subject: [PATCH v3 1/2] of: of_node_get()/of_node_put() nodes held in phandle cache Date: Tue, 18 Dec 2018 11:40:02 -0800 Message-Id: <1545162003-11577-2-git-send-email-frowand.list@gmail.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1545162003-11577-1-git-send-email-frowand.list@gmail.com> References: <1545162003-11577-1-git-send-email-frowand.list@gmail.com> X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: devicetree@vger.kernel.org, Thomas Falcon , linux-kernel@vger.kernel.org, Juliet Kim , Tyrel Datwyler Errors-To: linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org Sender: "Linuxppc-dev" From: Frank Rowand The phandle cache contains struct device_node pointers. The refcount of the pointers was not incremented while in the cache, allowing use after free error after kfree() of the node. Add the proper increment and decrement of the use count. Fixes: 0b3ce78e90fc ("of: cache phandle nodes to reduce cost of of_find_node_by_phandle()") Signed-off-by: Frank Rowand --- do not "cc: stable", unless the following commits are also in stable: commit e54192b48da7 ("of: fix phandle cache creation for DTs with no phandles") commit b9952b5218ad ("of: overlay: update phandle cache on overlay apply and remove") commit 0b3ce78e90fc ("of: cache phandle nodes to reduce cost of of_find_node_by_phandle()") drivers/of/base.c | 70 ++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 46 insertions(+), 24 deletions(-) diff --git a/drivers/of/base.c b/drivers/of/base.c index 09692c9b32a7..6c33d63361b8 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -116,9 +116,6 @@ int __weak of_node_to_nid(struct device_node *np) } #endif -static struct device_node **phandle_cache; -static u32 phandle_cache_mask; - /* * Assumptions behind phandle_cache implementation: * - phandle property values are in a contiguous range of 1..n @@ -127,6 +124,44 @@ int __weak of_node_to_nid(struct device_node *np) * - the phandle lookup overhead reduction provided by the cache * will likely be less */ + +static struct device_node **phandle_cache; +static u32 phandle_cache_mask; + +/* + * Caller must hold devtree_lock. + */ +static void __of_free_phandle_cache(void) +{ + u32 cache_entries = phandle_cache_mask + 1; + u32 k; + + if (!phandle_cache) + return; + + for (k = 0; k < cache_entries; k++) + of_node_put(phandle_cache[k]); + + kfree(phandle_cache); + phandle_cache = NULL; +} + +int of_free_phandle_cache(void) +{ + unsigned long flags; + + raw_spin_lock_irqsave(&devtree_lock, flags); + + __of_free_phandle_cache(); + + raw_spin_unlock_irqrestore(&devtree_lock, flags); + + return 0; +} +#if !defined(CONFIG_MODULES) +late_initcall_sync(of_free_phandle_cache); +#endif + void of_populate_phandle_cache(void) { unsigned long flags; @@ -136,8 +171,7 @@ void of_populate_phandle_cache(void) raw_spin_lock_irqsave(&devtree_lock, flags); - kfree(phandle_cache); - phandle_cache = NULL; + __of_free_phandle_cache(); for_each_of_allnodes(np) if (np->phandle && np->phandle != OF_PHANDLE_ILLEGAL) @@ -155,30 +189,15 @@ void of_populate_phandle_cache(void) goto out; for_each_of_allnodes(np) - if (np->phandle && np->phandle != OF_PHANDLE_ILLEGAL) + if (np->phandle && np->phandle != OF_PHANDLE_ILLEGAL) { + of_node_get(np); phandle_cache[np->phandle & phandle_cache_mask] = np; + } out: raw_spin_unlock_irqrestore(&devtree_lock, flags); } -int of_free_phandle_cache(void) -{ - unsigned long flags; - - raw_spin_lock_irqsave(&devtree_lock, flags); - - kfree(phandle_cache); - phandle_cache = NULL; - - raw_spin_unlock_irqrestore(&devtree_lock, flags); - - return 0; -} -#if !defined(CONFIG_MODULES) -late_initcall_sync(of_free_phandle_cache); -#endif - void __init of_core_init(void) { struct device_node *np; @@ -1195,8 +1214,11 @@ struct device_node *of_find_node_by_phandle(phandle handle) if (!np) { for_each_of_allnodes(np) if (np->phandle == handle) { - if (phandle_cache) + if (phandle_cache) { + /* will put when removed from cache */ + of_node_get(np); phandle_cache[masked_handle] = np; + } break; } } From patchwork Tue Dec 18 19:40:03 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Frank Rowand X-Patchwork-Id: 1015583 Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [203.11.71.2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 43K7rr4jhyz9s3q for ; Wed, 19 Dec 2018 06:49:32 +1100 (AEDT) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="EvdIx7sW"; dkim-atps=neutral Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 43K7rr0RQwzDqWQ for ; Wed, 19 Dec 2018 06:49:32 +1100 (AEDT) Authentication-Results: lists.ozlabs.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="EvdIx7sW"; dkim-atps=neutral X-Original-To: linuxppc-dev@lists.ozlabs.org Delivered-To: linuxppc-dev@lists.ozlabs.org Authentication-Results: lists.ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gmail.com (client-ip=2607:f8b0:4864:20::441; helo=mail-pf1-x441.google.com; envelope-from=frowand.list@gmail.com; receiver=) Authentication-Results: lists.ozlabs.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="EvdIx7sW"; dkim-atps=neutral Received: from mail-pf1-x441.google.com (mail-pf1-x441.google.com [IPv6:2607:f8b0:4864:20::441]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 43K7fN07sFzDqQW for ; Wed, 19 Dec 2018 06:40:27 +1100 (AEDT) Received: by mail-pf1-x441.google.com with SMTP id i12so8604327pfo.7 for ; Tue, 18 Dec 2018 11:40:27 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=QpdzDwSrsWE9LXF201TGBe/ML0bWOkO30Z8/kF7nPbI=; b=EvdIx7sWsnq7Qd2RW6u2Z4I/D44oPaP+GN0NKdh4xQu/pkczHfeotGbP3nFcVxvMsG OPcLpMUsddriRtjWdND7o4rcEx1DqEGmg0ILZRsfUAhC/nNiEustJdyjZjMb+aMkGIQU 3U9g13bmUnCk1GmBZMZ3XLoMc4W1NT2Ck+P2R2NbV9BDbLLir4KtUzwyBAANz8QRk3+Y mZiPI4TZ497fQaSLNk9bNZWlkkigj6xXjwTcBy9dvxJAEtZ5k8a0GQ37UrTUhl/fXF4/ /WPO1QykFrvQEAEY6qhkucoLrA3jQZk7zpU5KS42Afdm7PTRHHA1WTBGDBcilmlmxX8K slRA== 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=QpdzDwSrsWE9LXF201TGBe/ML0bWOkO30Z8/kF7nPbI=; b=WdFU+3zzbpr1h9QWhZK6ysO8YnQgN2uzCr8mObnQmQ4lngH0tt7fJpFMRPACnUdlWD 81SuOKWQP0M34arK+kZi5qZwWlmyzoVW82SoNxy3Ywrfey3W9I8QMBukYcVD9cEyy4IO kQ0Oj3eZHKzLdb3pmreGhrBNQXQTS6iqqzYt2J3gM4nhEMU8NY3ovYxn0tOknInVlqLU hGDkL3xVU8kj0KhV+zFCd1wwBs41nacfUGRdzcbmpCGN9nKNovGXusE2Vj5wAq55R6Fs +eAnmP++gSib8zbMpXP2sUiQeS779U9S9EpOwqX6qVplNUGfodnNTbuWY1P6nzCXAobx +d5A== X-Gm-Message-State: AA+aEWbjNppUPYo2CBVciAm/ASd6YB7AyluszTPJa7lUnnSMf3BXANmS dZy121P6tAr66Upk4bQH6Dg= X-Google-Smtp-Source: AFSGD/X7r9rnl1lgUNF7y1OGGMPqwJhmkVLN4/TzO8AAq32RmiBacasxsRfARiAyiD939uyB5i3dhg== X-Received: by 2002:a63:4b60:: with SMTP id k32mr422078pgl.186.1545162025672; Tue, 18 Dec 2018 11:40:25 -0800 (PST) Received: from localhost.localdomain (c-24-6-192-50.hsd1.ca.comcast.net. [24.6.192.50]) by smtp.gmail.com with ESMTPSA id b202sm29493069pfb.88.2018.12.18.11.40.24 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Tue, 18 Dec 2018 11:40:25 -0800 (PST) From: frowand.list@gmail.com To: robh+dt@kernel.org, Michael Bringmann , linuxppc-dev@lists.ozlabs.org Subject: [PATCH v3 2/2] of: __of_detach_node() - remove node from phandle cache Date: Tue, 18 Dec 2018 11:40:03 -0800 Message-Id: <1545162003-11577-3-git-send-email-frowand.list@gmail.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1545162003-11577-1-git-send-email-frowand.list@gmail.com> References: <1545162003-11577-1-git-send-email-frowand.list@gmail.com> X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: devicetree@vger.kernel.org, Thomas Falcon , linux-kernel@vger.kernel.org, Juliet Kim , Tyrel Datwyler Errors-To: linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org Sender: "Linuxppc-dev" From: Frank Rowand Non-overlay dynamic devicetree node removal may leave the node in the phandle cache. Subsequent calls to of_find_node_by_phandle() will incorrectly find the stale entry. Remove the node from the cache. Add paranoia checks in of_find_node_by_phandle() as a second level of defense (do not return cached node if detached, do not add node to cache if detached). Fixes: 0b3ce78e90fc ("of: cache phandle nodes to reduce cost of of_find_node_by_phandle()") Reported-by: Michael Bringmann Signed-off-by: Frank Rowand --- do not "cc: stable", unless the following commits are also in stable: commit e54192b48da7 ("of: fix phandle cache creation for DTs with no phandles") commit b9952b5218ad ("of: overlay: update phandle cache on overlay apply and remove") commit 0b3ce78e90fc ("of: cache phandle nodes to reduce cost of of_find_node_by_phandle()") Changes since v2: - add temporary variable np in __of_free_phandle_cache_entry() to improve readability - explain reason for WARN_ON() in comment - add Fixes tag in patch comment drivers/of/base.c | 31 ++++++++++++++++++++++++++++++- drivers/of/dynamic.c | 3 +++ drivers/of/of_private.h | 4 ++++ 3 files changed, 37 insertions(+), 1 deletion(-) diff --git a/drivers/of/base.c b/drivers/of/base.c index 6c33d63361b8..6d20b6dcf034 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -162,6 +162,28 @@ int of_free_phandle_cache(void) late_initcall_sync(of_free_phandle_cache); #endif +/* + * Caller must hold devtree_lock. + */ +void __of_free_phandle_cache_entry(phandle handle) +{ + phandle masked_handle; + struct device_node *np; + + if (!handle) + return; + + masked_handle = handle & phandle_cache_mask; + + if (phandle_cache) { + np = phandle_cache[masked_handle]; + if (np && handle == np->phandle) { + of_node_put(np); + phandle_cache[masked_handle] = NULL; + } + } +} + void of_populate_phandle_cache(void) { unsigned long flags; @@ -1209,11 +1231,18 @@ struct device_node *of_find_node_by_phandle(phandle handle) if (phandle_cache[masked_handle] && handle == phandle_cache[masked_handle]->phandle) np = phandle_cache[masked_handle]; + if (np && of_node_check_flag(np, OF_DETACHED)) { + WARN_ON(1); /* did not uncache np on node removal */ + of_node_put(np); + phandle_cache[masked_handle] = NULL; + np = NULL; + } } if (!np) { for_each_of_allnodes(np) - if (np->phandle == handle) { + if (np->phandle == handle && + !of_node_check_flag(np, OF_DETACHED)) { if (phandle_cache) { /* will put when removed from cache */ of_node_get(np); diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c index f4f8ed9b5454..ecea92f68c87 100644 --- a/drivers/of/dynamic.c +++ b/drivers/of/dynamic.c @@ -268,6 +268,9 @@ void __of_detach_node(struct device_node *np) } of_node_set_flag(np, OF_DETACHED); + + /* race with of_find_node_by_phandle() prevented by devtree_lock */ + __of_free_phandle_cache_entry(np->phandle); } /** diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h index 5d1567025358..24786818e32e 100644 --- a/drivers/of/of_private.h +++ b/drivers/of/of_private.h @@ -84,6 +84,10 @@ static inline void __of_detach_node_sysfs(struct device_node *np) {} int of_resolve_phandles(struct device_node *tree); #endif +#if defined(CONFIG_OF_DYNAMIC) +void __of_free_phandle_cache_entry(phandle handle); +#endif + #if defined(CONFIG_OF_OVERLAY) void of_overlay_mutex_lock(void); void of_overlay_mutex_unlock(void);