[PATCH 2/2] of: __of_detach_node() - remove node from phandle cache

From: frowand . list
Date: Fri Dec 14 2018 - 01:43:21 EST


From: Frank Rowand <frank.rowand@xxxxxxxx>

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 <mwb@xxxxxxxxxxxxxxxxxx>
Signed-off-by: Frank Rowand <frank.rowand@xxxxxxxx>
---
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);
--
Frank Rowand <frank.rowand@xxxxxxxx>