'__free(device_node)' attached to remote will cause a double release:
> OF: ERROR: of_node_release() detected bad of_node_put() on
> /soc@0/bus@32c00000/dsi@32e60000
In case remote is to be returned it must not be cleaned up by a call
to of_node_put. The caller has to do that as the documentation mentions.
Partly revert commit b94d24c08ee1a ("of: property: Remove calls to
of_node_put") to fix it.
Fixes: b94d24c08ee1 ("of: property: Remove calls to of_node_put")
Signed-off-by: Alexander Stein <[email protected]>
---
I get the following trace starting with next-20240529:
OF: ERROR: of_node_release() detected bad of_node_put() on /soc@0/bus@32c00000/dsi@32e60000
CPU: 0 PID: 11 Comm: kworker/u16:0 Not tainted 6.10.0-rc1-next-20240529+ #2511 610eecbb638fc5fab9a27e01b78203e8a379a280
Hardware name: TQ-Systems i.MX8MPlus TQMa8MPxL on MBa8MPxL (DT)
Workqueue: events_unbound deferred_probe_work_func
Call trace:
dump_backtrace+0x90/0x10c
show_stack+0x14/0x1c
dump_stack_lvl+0x6c/0x80
dump_stack+0x14/0x1c
of_node_release+0x11c/0x188
kobject_cleanup+0x48/0x17c
kobject_put+0x78/0xc0
of_node_put+0x14/0x20
drm_of_find_panel_or_bridge+0x84/0xd4
devm_drm_of_get_bridge+0x3c/0x8c
lcdif_attach_bridge+0x88/0x270
lcdif_load+0x144/0x28c
lcdif_probe+0x34/0xcc
platform_probe+0x64/0xe8
really_probe+0xc8/0x3ac
__driver_probe_device+0x84/0x188
driver_probe_device+0x38/0x150
__device_attach_driver+0xcc/0x194
bus_for_each_drv+0x80/0xdc
__device_attach+0x9c/0x1d0
device_initial_probe+0x10/0x18
bus_probe_device+0xa4/0xa8
deferred_probe_work_func+0x9c/0xe8
process_one_work+0x154/0x3fc
worker_thread+0x2f4/0x404
kthread+0xf4/0x100
ret_from_fork+0x10/0x20
OF: ERROR: next of_node_put() on this node will result in a kobject warning 'refcount_t: underflow; use-after-free.'
------------[ cut here ]------------
refcount_t: addition on 0; use-after-free.
WARNING: CPU: 0 PID: 11 at lib/refcount.c:25 refcount_warn_saturate+0x150/0x214
Modules linked in:
CPU: 0 PID: 11 Comm: kworker/u16:0 Not tainted 6.10.0-rc1-next-20240529+ #2511 610eecbb638fc5fab9a27e01b78203e8a379a280
Hardware name: TQ-Systems i.MX8MPlus TQMa8MPxL on MBa8MPxL (DT)
[...]
drivers/of/property.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/of/property.c b/drivers/of/property.c
index 83536216ed4fe..e5c6301643e4c 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -836,9 +836,7 @@ struct device_node *of_graph_get_remote_node(const struct device_node *node,
{
struct device_node *endpoint_node __free(device_node) =
of_graph_get_endpoint_by_regs(node, port, endpoint);
-
- struct device_node *remote __free(device_node) =
- of_graph_get_remote_port_parent(endpoint_node);
+ struct device_node *remote;
if (!endpoint_node) {
pr_debug("no valid endpoint (%d, %d) for node %pOF\n",
@@ -846,6 +844,7 @@ struct device_node *of_graph_get_remote_node(const struct device_node *node,
return NULL;
}
+ remote = of_graph_get_remote_port_parent(endpoint_node);
if (!remote) {
pr_debug("no valid remote node\n");
return NULL;
@@ -853,6 +852,7 @@ struct device_node *of_graph_get_remote_node(const struct device_node *node,
if (!of_device_is_available(remote)) {
pr_debug("not available for remote node\n");
+ of_node_put(remote);
return NULL;
}
--
2.34.1
On 29/05/2024 09:32, Alexander Stein wrote:
> '__free(device_node)' attached to remote will cause a double release:
>> OF: ERROR: of_node_release() detected bad of_node_put() on
>> /soc@0/bus@32c00000/dsi@32e60000
>
> In case remote is to be returned it must not be cleaned up by a call
> to of_node_put. The caller has to do that as the documentation mentions.
> Partly revert commit b94d24c08ee1a ("of: property: Remove calls to
> of_node_put") to fix it.
>
> Fixes: b94d24c08ee1 ("of: property: Remove calls to of_node_put")
> Signed-off-by: Alexander Stein <[email protected]>
> ---
> I get the following trace starting with next-20240529:
>
> OF: ERROR: of_node_release() detected bad of_node_put() on /soc@0/bus@32c00000/dsi@32e60000
> CPU: 0 PID: 11 Comm: kworker/u16:0 Not tainted 6.10.0-rc1-next-20240529+ #2511 610eecbb638fc5fab9a27e01b78203e8a379a280
> Hardware name: TQ-Systems i.MX8MPlus TQMa8MPxL on MBa8MPxL (DT)
> Workqueue: events_unbound deferred_probe_work_func
> Call trace:
> dump_backtrace+0x90/0x10c
> show_stack+0x14/0x1c
> dump_stack_lvl+0x6c/0x80
> dump_stack+0x14/0x1c
> of_node_release+0x11c/0x188
> kobject_cleanup+0x48/0x17c
> kobject_put+0x78/0xc0
> of_node_put+0x14/0x20
> drm_of_find_panel_or_bridge+0x84/0xd4
> devm_drm_of_get_bridge+0x3c/0x8c
> lcdif_attach_bridge+0x88/0x270
> lcdif_load+0x144/0x28c
> lcdif_probe+0x34/0xcc
> platform_probe+0x64/0xe8
> really_probe+0xc8/0x3ac
> __driver_probe_device+0x84/0x188
> driver_probe_device+0x38/0x150
> __device_attach_driver+0xcc/0x194
> bus_for_each_drv+0x80/0xdc
> __device_attach+0x9c/0x1d0
> device_initial_probe+0x10/0x18
> bus_probe_device+0xa4/0xa8
> deferred_probe_work_func+0x9c/0xe8
> process_one_work+0x154/0x3fc
> worker_thread+0x2f4/0x404
> kthread+0xf4/0x100
> ret_from_fork+0x10/0x20
> OF: ERROR: next of_node_put() on this node will result in a kobject warning 'refcount_t: underflow; use-after-free.'
> ------------[ cut here ]------------
> refcount_t: addition on 0; use-after-free.
> WARNING: CPU: 0 PID: 11 at lib/refcount.c:25 refcount_warn_saturate+0x150/0x214
> Modules linked in:
> CPU: 0 PID: 11 Comm: kworker/u16:0 Not tainted 6.10.0-rc1-next-20240529+ #2511 610eecbb638fc5fab9a27e01b78203e8a379a280
> Hardware name: TQ-Systems i.MX8MPlus TQMa8MPxL on MBa8MPxL (DT)
> [...]
>
> drivers/of/property.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/of/property.c b/drivers/of/property.c
> index 83536216ed4fe..e5c6301643e4c 100644
> --- a/drivers/of/property.c
> +++ b/drivers/of/property.c
> @@ -836,9 +836,7 @@ struct device_node *of_graph_get_remote_node(const struct device_node *node,
> {
> struct device_node *endpoint_node __free(device_node) =
> of_graph_get_endpoint_by_regs(node, port, endpoint);
> -
> - struct device_node *remote __free(device_node) =
> - of_graph_get_remote_port_parent(endpoint_node);
> + struct device_node *remote;
>
> if (!endpoint_node) {
> pr_debug("no valid endpoint (%d, %d) for node %pOF\n",
> @@ -846,6 +844,7 @@ struct device_node *of_graph_get_remote_node(const struct device_node *node,
> return NULL;
> }
>
> + remote = of_graph_get_remote_port_parent(endpoint_node);
> if (!remote) {
> pr_debug("no valid remote node\n");
> return NULL;
> @@ -853,6 +852,7 @@ struct device_node *of_graph_get_remote_node(const struct device_node *node,
>
> if (!of_device_is_available(remote)) {
> pr_debug("not available for remote node\n");
> + of_node_put(remote);
> return NULL;
> }
>
Tested-by: Neil Armstrong <[email protected]> # on SM8650-QRD
On Wed, May 29, 2024 at 2:33 AM Alexander Stein
<[email protected]> wrote:
>
> '__free(device_node)' attached to remote will cause a double release:
> > OF: ERROR: of_node_release() detected bad of_node_put() on
> > /soc@0/bus@32c00000/dsi@32e60000
>
> In case remote is to be returned it must not be cleaned up by a call
> to of_node_put. The caller has to do that as the documentation mentions.
> Partly revert commit b94d24c08ee1a ("of: property: Remove calls to
> of_node_put") to fix it.
>
> Fixes: b94d24c08ee1 ("of: property: Remove calls to of_node_put")
I've dropped it now.
> Signed-off-by: Alexander Stein <[email protected]>
> ---
> I get the following trace starting with next-20240529:
>
> OF: ERROR: of_node_release() detected bad of_node_put() on /soc@0/bus@32c00000/dsi@32e60000
> CPU: 0 PID: 11 Comm: kworker/u16:0 Not tainted 6.10.0-rc1-next-20240529+ #2511 610eecbb638fc5fab9a27e01b78203e8a379a280
> Hardware name: TQ-Systems i.MX8MPlus TQMa8MPxL on MBa8MPxL (DT)
> Workqueue: events_unbound deferred_probe_work_func
> Call trace:
> dump_backtrace+0x90/0x10c
> show_stack+0x14/0x1c
> dump_stack_lvl+0x6c/0x80
> dump_stack+0x14/0x1c
> of_node_release+0x11c/0x188
> kobject_cleanup+0x48/0x17c
> kobject_put+0x78/0xc0
> of_node_put+0x14/0x20
> drm_of_find_panel_or_bridge+0x84/0xd4
> devm_drm_of_get_bridge+0x3c/0x8c
> lcdif_attach_bridge+0x88/0x270
> lcdif_load+0x144/0x28c
> lcdif_probe+0x34/0xcc
> platform_probe+0x64/0xe8
> really_probe+0xc8/0x3ac
> __driver_probe_device+0x84/0x188
> driver_probe_device+0x38/0x150
> __device_attach_driver+0xcc/0x194
> bus_for_each_drv+0x80/0xdc
> __device_attach+0x9c/0x1d0
> device_initial_probe+0x10/0x18
> bus_probe_device+0xa4/0xa8
> deferred_probe_work_func+0x9c/0xe8
> process_one_work+0x154/0x3fc
> worker_thread+0x2f4/0x404
> kthread+0xf4/0x100
> ret_from_fork+0x10/0x20
> OF: ERROR: next of_node_put() on this node will result in a kobject warning 'refcount_t: underflow; use-after-free.'
> ------------[ cut here ]------------
> refcount_t: addition on 0; use-after-free.
> WARNING: CPU: 0 PID: 11 at lib/refcount.c:25 refcount_warn_saturate+0x150/0x214
> Modules linked in:
> CPU: 0 PID: 11 Comm: kworker/u16:0 Not tainted 6.10.0-rc1-next-20240529+ #2511 610eecbb638fc5fab9a27e01b78203e8a379a280
> Hardware name: TQ-Systems i.MX8MPlus TQMa8MPxL on MBa8MPxL (DT)
> [...]
>
> drivers/of/property.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/of/property.c b/drivers/of/property.c
> index 83536216ed4fe..e5c6301643e4c 100644
> --- a/drivers/of/property.c
> +++ b/drivers/of/property.c
> @@ -836,9 +836,7 @@ struct device_node *of_graph_get_remote_node(const struct device_node *node,
> {
> struct device_node *endpoint_node __free(device_node) =
> of_graph_get_endpoint_by_regs(node, port, endpoint);
> -
> - struct device_node *remote __free(device_node) =
> - of_graph_get_remote_port_parent(endpoint_node);
> + struct device_node *remote;
>
> if (!endpoint_node) {
> pr_debug("no valid endpoint (%d, %d) for node %pOF\n",
> @@ -846,6 +844,7 @@ struct device_node *of_graph_get_remote_node(const struct device_node *node,
> return NULL;
> }
>
> + remote = of_graph_get_remote_port_parent(endpoint_node);
> if (!remote) {
> pr_debug("no valid remote node\n");
> return NULL;
> @@ -853,6 +852,7 @@ struct device_node *of_graph_get_remote_node(const struct device_node *node,
>
> if (!of_device_is_available(remote)) {
> pr_debug("not available for remote node\n");
> + of_node_put(remote);
> return NULL;
> }
>
> --
> 2.34.1
>