diff mbox series

[v2,01/13] of: dynamic: Do not use "%pOF" while holding devtree_lock

Message ID c593d8389352c574b5be69d4ca4810da13326a50.1690533838.git.geert+renesas@glider.be (mailing list archive)
State Superseded
Delegated to: Geert Uytterhoeven
Headers show
Series of: overlay/unittest: Miscellaneous fixes and improvements | expand

Commit Message

Geert Uytterhoeven July 28, 2023, 8:50 a.m. UTC
While originally it was fine to format strings using "%pOF" while
holding devtree_lock, this now causes a deadlock.  Lockdep reports:

    of_get_parent from of_fwnode_get_parent+0x18/0x24
    ^^^^^^^^^^^^^
    of_fwnode_get_parent from fwnode_count_parents+0xc/0x28
    fwnode_count_parents from fwnode_full_name_string+0x18/0xac
    fwnode_full_name_string from device_node_string+0x1a0/0x404
    device_node_string from pointer+0x3c0/0x534
    pointer from vsnprintf+0x248/0x36c
    vsnprintf from vprintk_store+0x130/0x3b4

Fix this by making the locking cover only the parts that really need it.

Fixes: a92eb7621b9fb2c2 ("lib/vsprintf: Make use of fwnode API to obtain node names and separators")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - Correct fixes tag and update description.
---
 drivers/of/dynamic.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)
diff mbox series

Patch

diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index e311d406b1705306..eae45a1c673ee05f 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -601,13 +601,16 @@  static int __of_changeset_entry_apply(struct of_changeset_entry *ce)
 
 	__of_changeset_entry_dump(ce);
 
-	raw_spin_lock_irqsave(&devtree_lock, flags);
 	switch (ce->action) {
 	case OF_RECONFIG_ATTACH_NODE:
+		raw_spin_lock_irqsave(&devtree_lock, flags);
 		__of_attach_node(ce->np);
+		raw_spin_unlock_irqrestore(&devtree_lock, flags);
 		break;
 	case OF_RECONFIG_DETACH_NODE:
+		raw_spin_lock_irqsave(&devtree_lock, flags);
 		__of_detach_node(ce->np);
+		raw_spin_unlock_irqrestore(&devtree_lock, flags);
 		break;
 	case OF_RECONFIG_ADD_PROPERTY:
 		/* If the property is in deadprops then it must be removed */
@@ -619,7 +622,9 @@  static int __of_changeset_entry_apply(struct of_changeset_entry *ce)
 			}
 		}
 
+		raw_spin_lock_irqsave(&devtree_lock, flags);
 		ret = __of_add_property(ce->np, ce->prop);
+		raw_spin_unlock_irqrestore(&devtree_lock, flags);
 		if (ret) {
 			pr_err("changeset: add_property failed @%pOF/%s\n",
 				ce->np,
@@ -628,7 +633,9 @@  static int __of_changeset_entry_apply(struct of_changeset_entry *ce)
 		}
 		break;
 	case OF_RECONFIG_REMOVE_PROPERTY:
+		raw_spin_lock_irqsave(&devtree_lock, flags);
 		ret = __of_remove_property(ce->np, ce->prop);
+		raw_spin_unlock_irqrestore(&devtree_lock, flags);
 		if (ret) {
 			pr_err("changeset: remove_property failed @%pOF/%s\n",
 				ce->np,
@@ -647,7 +654,9 @@  static int __of_changeset_entry_apply(struct of_changeset_entry *ce)
 			}
 		}
 
+		raw_spin_lock_irqsave(&devtree_lock, flags);
 		ret = __of_update_property(ce->np, ce->prop, &old_prop);
+		raw_spin_unlock_irqrestore(&devtree_lock, flags);
 		if (ret) {
 			pr_err("changeset: update_property failed @%pOF/%s\n",
 				ce->np,
@@ -658,7 +667,6 @@  static int __of_changeset_entry_apply(struct of_changeset_entry *ce)
 	default:
 		ret = -EINVAL;
 	}
-	raw_spin_unlock_irqrestore(&devtree_lock, flags);
 
 	if (ret)
 		return ret;