Skip to contentSkip to conversation

driver/base/node: remove unnecessary kfree of node struct from unregister_one_node

PATCH In review Yasuaki Ishimatsu 2 participants 3 messages Oct 3, 2014 – Oct 9, 2014

Subsystems (3)
Stable branch Memory management Driver core
lore
Share & tools
lore

Yasuaki Ishimatsu

author
#0 lore raw

PATCH 1/1 driver/base/node: remove unnecessary kfree of node struct from unregister_one_node

Commit 92d585ef067d ("numa: fix NULL pointer access and memory leak in unregister_one_node()") added kfree() of node struct in unregister_one_node(). But node struct is freed by node_device_release() which is called in unregister_node(). So by adding the kfree(), node struct is freed two times.

While hot removing memory, the commit leads the following BUG_ON():

kernel BUG at mm/slub.c:3346!
invalid opcode: 0000 [#1] SMP
[...]

  Call Trace:
   [...] unregister_one_node
   [...] try_offline_node
   [...] remove_memory
   [...] acpi_memory_device_remove
   [...] acpi_bus_trim
   [...] acpi_bus_trim
   [...] acpi_device_hotplug
   [...] acpi_hotplug_work_fn
   [...] process_one_work
   [...] worker_thread
   [...] ? rescuer_thread
   [...] kthread
   [...] ? kthread_create_on_node
   [...] ret_from_fork
   [...] ? kthread_create_on_node

This patch removes unnecessary kfree() from unregister_one_node().

Signed-off-by Yasuaki Ishimatsu <[email protected]>
Cc Xishi Qiu <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Cc Andrew Morton <[email protected]>
Cc [email protected] # v3.16+
Fixes 92d585ef067d "numa: fix NULL pointer access and memory leak in unregister_one_node()"
---
 drivers/base/node.c | 1 -
 1 file changed, 1 deletion(-)
drivers/base/node.c +0 -1
index c6d3ae0..d51c49c 100644
@@ -603,7 +603,6 @@ void unregister_one_node(int nid)
 		return;
 
 	unregister_node(node_devices[nid]);
-	kfree(node_devices[nid]);
 	node_devices[nid] = NULL;
 }
Signature · 1 line
1.8.3.1
#1 lore raw
Quoted 4 lines · Yasuaki Ishimatsu

Commit 92d585ef067d ("numa: fix NULL pointer access and memory leak in unregister_one_node()") added kfree() of node struct in unregister_one_node(). But node struct is freed by node_device_release() which is called in unregister_node(). So by adding the kfree(),

Hi,

Is this path?
unregister_node()
device_unregister()
device_del()
bus_remove_device()
device_release_driver()
__device_release_driver()
devres_release_all()
release_nodes()
dr->node.release(dev, dr->data);
then which function is be called?

Thanks,
Xishi Qiu

Quoted 48 lines

node struct is freed two times.

While hot removing memory, the commit leads the following BUG_ON():

kernel BUG at mm/slub.c:3346!
invalid opcode: 0000 [#1] SMP
[...]

  Call Trace:
   [...] unregister_one_node
   [...] try_offline_node
   [...] remove_memory
   [...] acpi_memory_device_remove
   [...] acpi_bus_trim
   [...] acpi_bus_trim
   [...] acpi_device_hotplug
   [...] acpi_hotplug_work_fn
   [...] process_one_work
   [...] worker_thread
   [...] ? rescuer_thread
   [...] kthread
   [...] ? kthread_create_on_node
   [...] ret_from_fork
   [...] ? kthread_create_on_node

This patch removes unnecessary kfree() from unregister_one_node().

Signed-off-by Yasuaki Ishimatsu <[email protected]>
Cc Xishi Qiu <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Cc Andrew Morton <[email protected]>
Cc [email protected] # v3.16+
Fixes 92d585ef067d "numa: fix NULL pointer access and memory leak in unregister_one_node()"
---
 drivers/base/node.c | 1 -
 1 file changed, 1 deletion(-)
drivers/base/node.c +0 -1
index c6d3ae0..d51c49c 100644
@@ -603,7 +603,6 @@ void unregister_one_node(int nid)
 		return;
 
 	unregister_node(node_devices[nid]);
-	kfree(node_devices[nid]);
 	node_devices[nid] = NULL;
 }
author
#2 lore raw
Quoted 9 lines · Xishi Qiu
Quoted 4 lines · Yasuaki Ishimatsu

Commit 92d585ef067d ("numa: fix NULL pointer access and memory leak in unregister_one_node()") added kfree() of node struct in unregister_one_node(). But node struct is freed by node_device_release() which is called in unregister_node(). So by adding the kfree(),

Hi,

Quoted 11 lines

Is this path?
unregister_node()
device_unregister()
device_del()
bus_remove_device()
device_release_driver()
__device_release_driver()
devres_release_all()
release_nodes()
dr->node.release(dev, dr->data);
then which function is be called?

node_device_release is called as follows:

unregister_one_node()
-> unregister_node()
-> device_unregister()

    -> put_device()
      -> kobject_put()
        -> kref_put()
          -> kref_sub()
            -> kobject_release()
              -> kobject_cleanup()
                -> device_release()
                  -> node_device_release()

Thanks,
Yasuaki Ishimatsu

Quoted 55 lines

Thanks,
Xishi Qiu

Quoted 48 lines

node struct is freed two times.

While hot removing memory, the commit leads the following BUG_ON():

   kernel BUG at mm/slub.c:3346!
   invalid opcode: 0000 [#1] SMP
   [...]
   Call Trace:
    [...] unregister_one_node
    [...] try_offline_node
    [...] remove_memory
    [...] acpi_memory_device_remove
    [...] acpi_bus_trim
    [...] acpi_bus_trim
    [...] acpi_device_hotplug
    [...] acpi_hotplug_work_fn
    [...] process_one_work
    [...] worker_thread
    [...] ? rescuer_thread
    [...] kthread
    [...] ? kthread_create_on_node
    [...] ret_from_fork
    [...] ? kthread_create_on_node

This patch removes unnecessary kfree() from unregister_one_node().

Signed-off-by Yasuaki Ishimatsu <[email protected]>
Cc Xishi Qiu <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Cc Andrew Morton <[email protected]>
Cc [email protected] # v3.16+
Fixes 92d585ef067d "numa: fix NULL pointer access and memory leak in unregister_one_node()"
---
  drivers/base/node.c | 1 -
  1 file changed, 1 deletion(-)
drivers/base/node.c +0 -1
index c6d3ae0..d51c49c 100644
@@ -603,7 +603,6 @@ void unregister_one_node(int nid)
  		return;
 
  	unregister_node(node_devices[nid]);
-	kfree(node_devices[nid]);
  	node_devices[nid] = NULL;
  }