From patchwork Fri Dec 14 06:42:50 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Frank Rowand X-Patchwork-Id: 1013366 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)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 43GLhP1lwdz9sB5 for ; Fri, 14 Dec 2018 17:47:33 +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="IlhK6C1G"; 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 43GLhP01mjzDqxW for ; Fri, 14 Dec 2018 17:47:33 +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="IlhK6C1G"; 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::541; helo=mail-pg1-x541.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="IlhK6C1G"; dkim-atps=neutral Received: from mail-pg1-x541.google.com (mail-pg1-x541.google.com [IPv6:2607:f8b0:4864:20::541]) (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 43GLbL4RsbzDqvY for ; Fri, 14 Dec 2018 17:43:10 +1100 (AEDT) Received: by mail-pg1-x541.google.com with SMTP id n2so2249517pgm.3 for ; Thu, 13 Dec 2018 22:43:10 -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=SrNwE1dsifW+PsaKdjrVC/A0/dZpx7t1MCORIh/eIU4=; b=IlhK6C1G2zL+Jchbrki+gADoEfLEGQwDdkBAEDDgoHh3Wm4UUyLD97TEo3XpX9xBmk +EnOcmsP/Tk9omSStXWxULRVWO+zi+GDI/JQwPt43fiASOUu7bcuLOUHmQM2QkThLsnC yKcIb7v7th3wz70me/Jntp//JVP2mu0Fp7FfiqgK8qQD67A7gTXyuQte7B2GL+Rn8tVH 3sZAfKrmQhOAgqCgRtTsCRlHcjT8D0Bo1T+2d/cYH96qEJmB4t8U8k9PIaa6NW+IdQbb +M4QnR/yXvIAihMN3uoi4YemCKvGOsvGAuaijkpeEC1HxPTLHKn/Ph1LbEx4ruiYmf0I 59OQ== 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=SrNwE1dsifW+PsaKdjrVC/A0/dZpx7t1MCORIh/eIU4=; b=jlFZSBYvs5lzz/q6sXCsbhZeZvJvKaXTzCd5b6uMT4zYk+hWHs+lT9dUQE42DWIsTp wKmur+HJK6QljID/8jcQ6YHw8WtB1vAv7YQOloYKWvXiaBYh5RQT5b9lXeVvXljaq0A8 ki8S/Pcj060flUHc+JS+q2bQkpFEIZlkE3QE3Ux+dnHTxkBIe4b1UOHxDr1n2Eu/MHqL 7M8kn6QDY9Ns/WvZWYleGoFZy2IoN10NH+t9Pe0TvOktgpCGeQZSXWM2YmCDl9zwXD02 Ax9MugPPY+DBZ2vtmJ/ERRMm54/2RXq0cf4N+kGERTrEumSTWm6i5gqwacWdj0E6WUTG Kz1g== X-Gm-Message-State: AA+aEWalDuzSm7+DRFaBAt8eYrY/80RvLHdjhVYNSVd6rHorwmMr4lo5 tNvqzUhLwNnlHPe6PHkac5Y= X-Google-Smtp-Source: AFSGD/VUR1Phk9+hCjoPmSSscts0gfWzBNqvNiX7YCaTZYJD+6GWeqlnJFWa221bH83qYP2CEL3S2w== X-Received: by 2002:a63:83c1:: with SMTP id h184mr1630881pge.437.1544769788811; Thu, 13 Dec 2018 22:43:08 -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 e16sm5132645pfn.46.2018.12.13.22.43.07 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 13 Dec 2018 22:43:08 -0800 (PST) From: frowand.list@gmail.com To: robh+dt@kernel.org, Michael Bringmann , linuxppc-dev@lists.ozlabs.org Subject: [PATCH 1/2] of: of_node_get()/of_node_put() nodes held in phandle cache Date: Thu, 13 Dec 2018 22:42:50 -0800 Message-Id: <1544769771-5468-2-git-send-email-frowand.list@gmail.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1544769771-5468-1-git-send-email-frowand.list@gmail.com> References: <1544769771-5468-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..d599367cb92a 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. + */ +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 Fri Dec 14 06:42:51 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Frank Rowand X-Patchwork-Id: 1013367 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)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 43GLl70RSpz9sBQ for ; Fri, 14 Dec 2018 17:49:55 +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="BfR4psKl"; 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 43GLl65lNvzDqw9 for ; Fri, 14 Dec 2018 17:49:54 +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="BfR4psKl"; 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::442; helo=mail-pf1-x442.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="BfR4psKl"; dkim-atps=neutral Received: from mail-pf1-x442.google.com (mail-pf1-x442.google.com [IPv6:2607:f8b0:4864:20::442]) (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 43GLbM61f1zDqvH for ; Fri, 14 Dec 2018 17:43:11 +1100 (AEDT) Received: by mail-pf1-x442.google.com with SMTP id q1so2352461pfi.5 for ; Thu, 13 Dec 2018 22:43:11 -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=Cf9GacyXSfcSYQD1hrpRVZxNJit6THuob2xtfEuPnDI=; b=BfR4psKlef8o1v7MrtDWgkZZzDExJYmy4mI4dQpM+NCUPR36QJKUy4unCEA57QnUUA Sps7DfOJ1jBmEfYhhCSRvMGVHvGYFb/NaYX9J5SEqeGEnOHBlNihiicX61pVD+T28TYs 4TNWbdVHqBicWWAIW/nkNhKqkqf1ldlMJ3WLyFWQB8MMWXMvSDs+reYLEKwrD0iVMTcq iSFJl0s30i76WyP/6mhp07Mw064q9uUVCjDzThKZd0Fh2H18oehZFVr+8s/f4SFm4zmT GWYzHI8o7MaYkIaXx7SA6AVW/SQ25KY8g9zbGr8eNhj7Onp5dm5QoOw031wSS8HFGDrj bdvQ== 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=Cf9GacyXSfcSYQD1hrpRVZxNJit6THuob2xtfEuPnDI=; b=SXwaab7M7yOGHphuf8zUWIfYR521lD3RSFR7GZRN0R6ViljodqKFOzldPxT52oV0Mb UXhq09uXgx0MreqhxJtz6UyAewbRtpNuKSRjszgy8oxrIPrG1TBaujnqUI/FDduQexOz oK0c+gkbtgHopEzCOr2vs/2wXsnd1GDNhsVOApODHEF+GklKagfuu0p84V96uhe2SwCe oBrx1fL3dRHBbd2JXDgovN8x5WT1DLfWrjneYnPX09h5wA1NqcVoOmlg/jDpOTQ1w5sU Fu61M2Uza44Pk83gMw1zKv+2kG2CIkuzDn41Az6+AO/2quP0nX0jmJcZRM2N4BeJKJZk boeA== X-Gm-Message-State: AA+aEWaEl7wTAj6QtHHIKLWGTnb4JEkH2WNiREBQqZL4zmGdsHnDcFmo TvXPTYd2wEH15R/R0waCnV8= X-Google-Smtp-Source: AFSGD/Vg2CvhlVmp4b//gn5A/j1f42Ky3SwA3E9gurSpJOgfuPyLKwTNxjQsnapNqgIr3rq+9M+qEA== X-Received: by 2002:a63:9501:: with SMTP id p1mr1676451pgd.149.1544769790012; Thu, 13 Dec 2018 22:43:10 -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 e16sm5132645pfn.46.2018.12.13.22.43.08 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 13 Dec 2018 22:43:09 -0800 (PST) From: frowand.list@gmail.com To: robh+dt@kernel.org, Michael Bringmann , linuxppc-dev@lists.ozlabs.org Subject: [PATCH 2/2] of: __of_detach_node() - remove node from phandle cache Date: Thu, 13 Dec 2018 22:42:51 -0800 Message-Id: <1544769771-5468-3-git-send-email-frowand.list@gmail.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1544769771-5468-1-git-send-email-frowand.list@gmail.com> References: <1544769771-5468-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). Reported-by: Michael Bringmann Signed-off-by: Frank Rowand --- drivers/of/base.c | 29 ++++++++++++++++++++++++++++- drivers/of/dynamic.c | 3 +++ drivers/of/of_private.h | 4 ++++ 3 files changed, 35 insertions(+), 1 deletion(-) diff --git a/drivers/of/base.c b/drivers/of/base.c index d599367cb92a..34a5125713c8 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -162,6 +162,27 @@ 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; + + if (!handle) + return; + + masked_handle = handle & phandle_cache_mask; + + if (phandle_cache) { + if (phandle_cache[masked_handle] && + handle == phandle_cache[masked_handle]->phandle) { + of_node_put(phandle_cache[masked_handle]); + phandle_cache[masked_handle] = NULL; + } + } +} + void of_populate_phandle_cache(void) { unsigned long flags; @@ -1209,11 +1230,17 @@ 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)) { + 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);