From patchwork Mon Dec 17 07:56:35 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Frank Rowand X-Patchwork-Id: 1014318 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 43JDBS6DMGz9sD9 for ; Mon, 17 Dec 2018 19:01:36 +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="KqHNmOrv"; 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 43JDBS4WyLzDqCM for ; Mon, 17 Dec 2018 19:01:36 +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="KqHNmOrv"; 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::641; helo=mail-pl1-x641.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="KqHNmOrv"; dkim-atps=neutral Received: from mail-pl1-x641.google.com (mail-pl1-x641.google.com [IPv6:2607:f8b0:4864:20::641]) (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 43JD545b2HzDqZl for ; Mon, 17 Dec 2018 18:56:56 +1100 (AEDT) Received: by mail-pl1-x641.google.com with SMTP id z23so5749553plo.0 for ; Sun, 16 Dec 2018 23:56:56 -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=IjwDbTS/z4KBFIXquKZ/wO5Kv02klbSOetli+9vOaNc=; b=KqHNmOrvGwcJtyxcs+bLRIXLWzVX36HkNJGLRrfUR3CEKBqXsklbYR8C/KA3IZlnWc FhT2iZq68uXcJra2JRQaCR/mofFsGze1c2WyGlMnVsWOYCo4Wcs1px01cXn7jc65rK0r vUUkqslHiEp2qW0jnK3exTlgppFkm+6Ef8gjP2dlsWvh7PtMAdgKML5jRGUFQ1+AAQiY qX64KCYho5j38we+2uf1hPVAo+CrPmCGBC49ZG4I28MFDJ4IMGOUiXB+tjZS4gABlODB TleAV3VEoo5GNiyJ8W7bTz/JLFcBuXoM3L0YgfaH8DRx1qOno+A9wSYov+WbgxISo3Yu 97nw== 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=IjwDbTS/z4KBFIXquKZ/wO5Kv02klbSOetli+9vOaNc=; b=OG6HKZGJsjod4SVz6Z3m0T/E16d1hY/razdd2Fev9kZekmLxJzHPEgQAEUlhpHtyiL UDP7rTe3C937m0Lwut6OVYijRSKBGM0FwEW1Dyb/27kvStnk40d6+LPO+Ect64pULjCT /uUTEsHsDrIhjcoDTozYr3nn+HioxYTVGILpuLQFi0/F8BY+1bL32BfvC3CCj0jFNkn8 YKoN9skSLuLuwpOIPUNJ2ivNCsgcxgFXS8D5rywbJAISklPAiJi0F8zB7OvKeRsDPQP/ GKHWa+kCdn3Zws7XnrjxFaENSguxbKFWp2BTWPKa/F+gHc3YnWpyuSgH4CMtWZ+FYCEk z+Jw== X-Gm-Message-State: AA+aEWbv2+i66VlzP3VOsibsR29espxj97ajqSMQ0Pt2WGiPsrQQRaRK /JZKa0yssvl1wF+lXmSNlJ0= X-Google-Smtp-Source: AFSGD/VzadQ7KllZWniZ0SFntweMpX+J7hhizCMBCQ81Z8FIwynVKUz+AnTwbjeju7NabFGK7OQK1g== X-Received: by 2002:a17:902:724a:: with SMTP id c10mr12061675pll.51.1545033414561; Sun, 16 Dec 2018 23:56:54 -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 r8sm14216610pgr.48.2018.12.16.23.56.53 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Sun, 16 Dec 2018 23:56:54 -0800 (PST) From: frowand.list@gmail.com To: robh+dt@kernel.org, Michael Bringmann , linuxppc-dev@lists.ozlabs.org Subject: [PATCH v2 1/2] of: of_node_get()/of_node_put() nodes held in phandle cache Date: Sun, 16 Dec 2018 23:56:35 -0800 Message-Id: <1545033396-24485-2-git-send-email-frowand.list@gmail.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1545033396-24485-1-git-send-email-frowand.list@gmail.com> References: <1545033396-24485-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 --- changes since v1 - make __of_free_phandle_cache() static 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 Mon Dec 17 07:56:36 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Frank Rowand X-Patchwork-Id: 1014319 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 43JDFJ1ZrLz9sD9 for ; Mon, 17 Dec 2018 19:04:04 +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="PsSxbB6w"; 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 43JDFH70mNzDqMF for ; Mon, 17 Dec 2018 19:04:03 +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="PsSxbB6w"; 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::543; helo=mail-pg1-x543.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="PsSxbB6w"; dkim-atps=neutral Received: from mail-pg1-x543.google.com (mail-pg1-x543.google.com [IPv6:2607:f8b0:4864:20::543]) (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 43JD560gwCzDqZQ for ; Mon, 17 Dec 2018 18:56:57 +1100 (AEDT) Received: by mail-pg1-x543.google.com with SMTP id w7so5703180pgp.13 for ; Sun, 16 Dec 2018 23:56:57 -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=au8EiM8byvG0y1Hv7QLp0mVhGQvk710Vc9STgbc0N48=; b=PsSxbB6wTE0H5YQel8vnGmdkBREnOOdX8oEXQV5qc4BPK9ICS3ZUIkqSzgfXDJWA9g P7zC+UyxG+QZb1UZXKBYY6kvIHG+2VJZY1OOc1dGLe0ImXR4Dx/huuuFSdCTaZspGQ9u Otqq8R0osb9hgOmUdLad3l200yl25gE5KIWQPVz/Hdbtqlrogx2nYoXkRaVOJ9wjCEyl JkvDF17KCT8l4v+F7TDAe7qkHLA37awgSAUUslMdCKROfYlH8v7Vpf+eYBq9xy9U17Iv MCZMPUb+PDDg1TiTWOuLJvB3H5qotK6oj/Y245vK/u6kpxd+e/vk60Irnv3BfbPs78xl hwVA== 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=au8EiM8byvG0y1Hv7QLp0mVhGQvk710Vc9STgbc0N48=; b=JkaZzRubhbN2GXYa97MQFGYuFyFdq9xaI/fre17dQ/hAPgOjrlI0JB0wSWAFnxzBXU Xlqq6ndP8pf/KGsSDG0riqE1nOAqpTnul9zK17DEr3NKRfXcYLU/UQTnly7DKteZYkUg HxCUGZBgV0NXYqfXGk3YnRngOMUjC75aGiY9ebglqHK7jvzFYLH8cNMQvK2sQtcBlyfH 4roYU//AGU3imCkdabIKlkWvP2zGkYPjVen0bbCsE1i9DH8tauSe7ww9oSmsg3M7wyvz cGMglNoqbMjGwaDI1liuZtL1YuzV85YsCrS/bCRpb9N0qo2ekox09fCifGfN8JoCE8oB A5eQ== X-Gm-Message-State: AA+aEWbqApUzgFpQnvJ86m6hvMA8cTqNNgAoSCHrAaIcZjvv1PXhlfUc 5z9KEoAt4L7Fl9T6lKOcmPQ= X-Google-Smtp-Source: AFSGD/VITzx/iT2E843sYjhHKQvhXFgUaF/GLt0spXhxUmG9dVpChMUCC1mF6fbGF5mDYuwFW11zVA== X-Received: by 2002:a62:220d:: with SMTP id i13mr11892705pfi.162.1545033415905; Sun, 16 Dec 2018 23:56:55 -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 r8sm14216610pgr.48.2018.12.16.23.56.54 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Sun, 16 Dec 2018 23:56:55 -0800 (PST) From: frowand.list@gmail.com To: robh+dt@kernel.org, Michael Bringmann , linuxppc-dev@lists.ozlabs.org Subject: [PATCH v2 2/2] of: __of_detach_node() - remove node from phandle cache Date: Sun, 16 Dec 2018 23:56:36 -0800 Message-Id: <1545033396-24485-3-git-send-email-frowand.list@gmail.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1545033396-24485-1-git-send-email-frowand.list@gmail.com> References: <1545033396-24485-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 --- changes since v1: - add WARN_ON(1) for unexpected condition in of_find_node_by_phandle() drivers/of/base.c | 30 +++++++++++++++++++++++++++++- drivers/of/dynamic.c | 3 +++ drivers/of/of_private.h | 4 ++++ 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/drivers/of/base.c b/drivers/of/base.c index 6c33d63361b8..ad71864cecf5 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,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); + 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);