Re: [PATCH 1/4] memory-hotplug: add memory_block_release
> It is not correct. The kobject_put() is prepared against find_memory_block() > in remove_memory_block() since kobject->kref is incremented in it. > So release_memory_block() is called by device_unregister() correctly as > follows: Ok. Looks good then. Please rewrite the description more kindly at next spin. Current one is really really bad. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] memory-hotplug: add memory_block_release
On 09/28/2012 11:45 AM, Yasuaki Ishimatsu wrote:
Hi Kosaki-san,
2012/09/28 10:35, KOSAKI Motohiro wrote:
On Thu, Sep 27, 2012 at 8:24 PM, Yasuaki Ishimatsu
wrote:
Hi Chen,
2012/09/27 19:20, Ni zhan Chen wrote:
Hi Congyang,
2012/9/27
From: Yasuaki Ishimatsu
When calling remove_memory_block(), the function shows following
message
at
device_release().
Device 'memory528' does not have a release() function, it is
broken and
must
be fixed.
What's the difference between the patch and original implemetation?
The implementation is for removing a memory_block. So the purpose is
same as original one. But original code is bad manner.
kobject_cleanup()
is called by remove_memory_block() at last. But release function for
releasing memory_block is not registered. As a result, the kernel
message
is shown. IMHO, memory_block should be release by the releae function.
but your patch introduced use after free bug, if i understand correctly.
See unregister_memory() function. After your patch, kobject_put() call
release_memory_block() and kfree(). and then device_unregister() will
touch freed memory.
It is not correct. The kobject_put() is prepared against
find_memory_block()
in remove_memory_block() since kobject->kref is incremented in it.
So release_memory_block() is called by device_unregister() correctly
as follows:
Another issue is memory hotplug which is not associated to this patch
report to you:
IIUC, function register_mem_sect_under_node should be renamed to
register_mem_block_under_node,
since this function is register memory block instead of memory section.
[ 1014.589008] Pid: 126, comm: kworker/0:2 Not tainted
3.6.0-rc3-enable-memory-hotremove-and-root-bridge #3
[ 1014.702437] Call Trace:
[ 1014.731684] [] release_memory_block+0x16/0x30
[ 1014.803581] [] device_release+0x27/0xa0
[ 1014.869312] [] kobject_cleanup+0x82/0x1b0
[ 1014.937062] [] kobject_release+0xd/0x10
[ 1015.002718] [] kobject_put+0x2c/0x60
[ 1015.065271] [] put_device+0x17/0x20
[ 1015.126794] [] device_unregister+0x2a/0x60
[ 1015.195578] [] remove_memory_block+0xbb/0xf0
[ 1015.266434] [] unregister_memory_section+0x1f/0x30
[ 1015.343532] [] __remove_section+0x68/0x110
[ 1015.412318] [] __remove_pages+0xe7/0x120
[ 1015.479021] [] arch_remove_memory+0x2c/0x80
[ 1015.548845] [] remove_memory+0x6b/0xd0
[ 1015.613474] []
acpi_memory_device_remove_memory+0x48/0x73
[ 1015.697834] [] acpi_memory_device_remove+0x2b/0x44
[ 1015.774922] [] acpi_device_remove+0x90/0xb2
[ 1015.844796] [] __device_release_driver+0x7c/0xf0
[ 1015.919814] [] device_release_driver+0x2f/0x50
[ 1015.992753] [] acpi_bus_remove+0x32/0x6d
[ 1016.059462] [] acpi_bus_trim+0x91/0x102
[ 1016.125128] []
acpi_bus_hot_remove_device+0x88/0x16b
[ 1016.204295] [] acpi_os_execute_deferred+0x27/0x34
[ 1016.280350] [] process_one_work+0x219/0x680
[ 1016.350173] [] ? process_one_work+0x1b8/0x680
[ 1016.422072] [] ?
acpi_os_wait_events_complete+0x23/0x23
[ 1016.504357] [] worker_thread+0x12e/0x320
[ 1016.571064] [] ? manage_workers+0x110/0x110
[ 1016.640886] [] kthread+0xc6/0xd0
[ 1016.699290] [] kernel_thread_helper+0x4/0x10
[ 1016.770149] [] ? retint_restore_args+0x13/0x13
[ 1016.843165] [] ? __init_kthread_worker+0x70/0x70
[ 1016.918200] [] ? gs_change+0x13/0x13
Thanks,
Yasuaki Ishimatsu
static void
unregister_memory(struct memory_block *memory)
{
BUG_ON(memory->dev.bus != _subsys);
/* drop the ref. we got in remove_memory_block() */
kobject_put(>dev.kobj);
device_unregister(>dev);
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] memory-hotplug: add memory_block_release
Hi Chen,
2012/09/28 15:04, Ni zhan Chen wrote:
On 09/28/2012 11:45 AM, Yasuaki Ishimatsu wrote:
Hi Kosaki-san,
2012/09/28 10:35, KOSAKI Motohiro wrote:
On Thu, Sep 27, 2012 at 8:24 PM, Yasuaki Ishimatsu
wrote:
Hi Chen,
2012/09/27 19:20, Ni zhan Chen wrote:
Hi Congyang,
2012/9/27
From: Yasuaki Ishimatsu
When calling remove_memory_block(), the function shows following message
at
device_release().
Device 'memory528' does not have a release() function, it is broken and
must
be fixed.
What's the difference between the patch and original implemetation?
The implementation is for removing a memory_block. So the purpose is
same as original one. But original code is bad manner. kobject_cleanup()
is called by remove_memory_block() at last. But release function for
releasing memory_block is not registered. As a result, the kernel message
is shown. IMHO, memory_block should be release by the releae function.
but your patch introduced use after free bug, if i understand correctly.
See unregister_memory() function. After your patch, kobject_put() call
release_memory_block() and kfree(). and then device_unregister() will
touch freed memory.
this patch is similiar to [RFC v9 PATCH 10/21] memory-hotplug: add
memory_block_release, they handle the same issue, can these two patches be fold
to one?
You're right. The patch is same as [RFC v9 PATCH 10/21].
The patch is a bug fix. So we separated it from memory-hotplug patch-set.
Thanks,
Yasuaki Ishimatsu
It is not correct. The kobject_put() is prepared against find_memory_block()
in remove_memory_block() since kobject->kref is incremented in it.
So release_memory_block() is called by device_unregister() correctly as follows:
[ 1014.589008] Pid: 126, comm: kworker/0:2 Not tainted
3.6.0-rc3-enable-memory-hotremove-and-root-bridge #3
[ 1014.702437] Call Trace:
[ 1014.731684] [] release_memory_block+0x16/0x30
[ 1014.803581] [] device_release+0x27/0xa0
[ 1014.869312] [] kobject_cleanup+0x82/0x1b0
[ 1014.937062] [] kobject_release+0xd/0x10
[ 1015.002718] [] kobject_put+0x2c/0x60
[ 1015.065271] [] put_device+0x17/0x20
[ 1015.126794] [] device_unregister+0x2a/0x60
[ 1015.195578] [] remove_memory_block+0xbb/0xf0
[ 1015.266434] [] unregister_memory_section+0x1f/0x30
[ 1015.343532] [] __remove_section+0x68/0x110
[ 1015.412318] [] __remove_pages+0xe7/0x120
[ 1015.479021] [] arch_remove_memory+0x2c/0x80
[ 1015.548845] [] remove_memory+0x6b/0xd0
[ 1015.613474] [] acpi_memory_device_remove_memory+0x48/0x73
[ 1015.697834] [] acpi_memory_device_remove+0x2b/0x44
[ 1015.774922] [] acpi_device_remove+0x90/0xb2
[ 1015.844796] [] __device_release_driver+0x7c/0xf0
[ 1015.919814] [] device_release_driver+0x2f/0x50
[ 1015.992753] [] acpi_bus_remove+0x32/0x6d
[ 1016.059462] [] acpi_bus_trim+0x91/0x102
[ 1016.125128] [] acpi_bus_hot_remove_device+0x88/0x16b
[ 1016.204295] [] acpi_os_execute_deferred+0x27/0x34
[ 1016.280350] [] process_one_work+0x219/0x680
[ 1016.350173] [] ? process_one_work+0x1b8/0x680
[ 1016.422072] [] ? acpi_os_wait_events_complete+0x23/0x23
[ 1016.504357] [] worker_thread+0x12e/0x320
[ 1016.571064] [] ? manage_workers+0x110/0x110
[ 1016.640886] [] kthread+0xc6/0xd0
[ 1016.699290] [] kernel_thread_helper+0x4/0x10
[ 1016.770149] [] ? retint_restore_args+0x13/0x13
[ 1016.843165] [] ? __init_kthread_worker+0x70/0x70
[ 1016.918200] [] ? gs_change+0x13/0x13
Thanks,
Yasuaki Ishimatsu
static void
unregister_memory(struct memory_block *memory)
{
BUG_ON(memory->dev.bus != _subsys);
/* drop the ref. we got in remove_memory_block() */
kobject_put(>dev.kobj);
device_unregister(>dev);
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] memory-hotplug: add memory_block_release
On 09/28/2012 11:45 AM, Yasuaki Ishimatsu wrote:
Hi Kosaki-san,
2012/09/28 10:35, KOSAKI Motohiro wrote:
On Thu, Sep 27, 2012 at 8:24 PM, Yasuaki Ishimatsu
wrote:
Hi Chen,
2012/09/27 19:20, Ni zhan Chen wrote:
Hi Congyang,
2012/9/27
From: Yasuaki Ishimatsu
When calling remove_memory_block(), the function shows following
message
at
device_release().
Device 'memory528' does not have a release() function, it is
broken and
must
be fixed.
What's the difference between the patch and original implemetation?
The implementation is for removing a memory_block. So the purpose is
same as original one. But original code is bad manner.
kobject_cleanup()
is called by remove_memory_block() at last. But release function for
releasing memory_block is not registered. As a result, the kernel
message
is shown. IMHO, memory_block should be release by the releae function.
but your patch introduced use after free bug, if i understand correctly.
See unregister_memory() function. After your patch, kobject_put() call
release_memory_block() and kfree(). and then device_unregister() will
touch freed memory.
this patch is similiar to [RFC v9 PATCH 10/21] memory-hotplug: add
memory_block_release, they handle the same issue, can these two patches
be fold to one?
It is not correct. The kobject_put() is prepared against
find_memory_block()
in remove_memory_block() since kobject->kref is incremented in it.
So release_memory_block() is called by device_unregister() correctly
as follows:
[ 1014.589008] Pid: 126, comm: kworker/0:2 Not tainted
3.6.0-rc3-enable-memory-hotremove-and-root-bridge #3
[ 1014.702437] Call Trace:
[ 1014.731684] [] release_memory_block+0x16/0x30
[ 1014.803581] [] device_release+0x27/0xa0
[ 1014.869312] [] kobject_cleanup+0x82/0x1b0
[ 1014.937062] [] kobject_release+0xd/0x10
[ 1015.002718] [] kobject_put+0x2c/0x60
[ 1015.065271] [] put_device+0x17/0x20
[ 1015.126794] [] device_unregister+0x2a/0x60
[ 1015.195578] [] remove_memory_block+0xbb/0xf0
[ 1015.266434] [] unregister_memory_section+0x1f/0x30
[ 1015.343532] [] __remove_section+0x68/0x110
[ 1015.412318] [] __remove_pages+0xe7/0x120
[ 1015.479021] [] arch_remove_memory+0x2c/0x80
[ 1015.548845] [] remove_memory+0x6b/0xd0
[ 1015.613474] []
acpi_memory_device_remove_memory+0x48/0x73
[ 1015.697834] [] acpi_memory_device_remove+0x2b/0x44
[ 1015.774922] [] acpi_device_remove+0x90/0xb2
[ 1015.844796] [] __device_release_driver+0x7c/0xf0
[ 1015.919814] [] device_release_driver+0x2f/0x50
[ 1015.992753] [] acpi_bus_remove+0x32/0x6d
[ 1016.059462] [] acpi_bus_trim+0x91/0x102
[ 1016.125128] []
acpi_bus_hot_remove_device+0x88/0x16b
[ 1016.204295] [] acpi_os_execute_deferred+0x27/0x34
[ 1016.280350] [] process_one_work+0x219/0x680
[ 1016.350173] [] ? process_one_work+0x1b8/0x680
[ 1016.422072] [] ?
acpi_os_wait_events_complete+0x23/0x23
[ 1016.504357] [] worker_thread+0x12e/0x320
[ 1016.571064] [] ? manage_workers+0x110/0x110
[ 1016.640886] [] kthread+0xc6/0xd0
[ 1016.699290] [] kernel_thread_helper+0x4/0x10
[ 1016.770149] [] ? retint_restore_args+0x13/0x13
[ 1016.843165] [] ? __init_kthread_worker+0x70/0x70
[ 1016.918200] [] ? gs_change+0x13/0x13
Thanks,
Yasuaki Ishimatsu
static void
unregister_memory(struct memory_block *memory)
{
BUG_ON(memory->dev.bus != _subsys);
/* drop the ref. we got in remove_memory_block() */
kobject_put(>dev.kobj);
device_unregister(>dev);
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] memory-hotplug: add memory_block_release
On 09/28/2012 11:45 AM, Yasuaki Ishimatsu wrote:
Hi Kosaki-san,
2012/09/28 10:35, KOSAKI Motohiro wrote:
On Thu, Sep 27, 2012 at 8:24 PM, Yasuaki Ishimatsu
isimatu.yasu...@jp.fujitsu.com wrote:
Hi Chen,
2012/09/27 19:20, Ni zhan Chen wrote:
Hi Congyang,
2012/9/27 we...@cn.fujitsu.com
From: Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com
When calling remove_memory_block(), the function shows following
message
at
device_release().
Device 'memory528' does not have a release() function, it is
broken and
must
be fixed.
What's the difference between the patch and original implemetation?
The implementation is for removing a memory_block. So the purpose is
same as original one. But original code is bad manner.
kobject_cleanup()
is called by remove_memory_block() at last. But release function for
releasing memory_block is not registered. As a result, the kernel
message
is shown. IMHO, memory_block should be release by the releae function.
but your patch introduced use after free bug, if i understand correctly.
See unregister_memory() function. After your patch, kobject_put() call
release_memory_block() and kfree(). and then device_unregister() will
touch freed memory.
this patch is similiar to [RFC v9 PATCH 10/21] memory-hotplug: add
memory_block_release, they handle the same issue, can these two patches
be fold to one?
It is not correct. The kobject_put() is prepared against
find_memory_block()
in remove_memory_block() since kobject-kref is incremented in it.
So release_memory_block() is called by device_unregister() correctly
as follows:
[ 1014.589008] Pid: 126, comm: kworker/0:2 Not tainted
3.6.0-rc3-enable-memory-hotremove-and-root-bridge #3
[ 1014.702437] Call Trace:
[ 1014.731684] [8144d096] release_memory_block+0x16/0x30
[ 1014.803581] [81438587] device_release+0x27/0xa0
[ 1014.869312] [8133e962] kobject_cleanup+0x82/0x1b0
[ 1014.937062] [8133ea9d] kobject_release+0xd/0x10
[ 1015.002718] [8133e7ec] kobject_put+0x2c/0x60
[ 1015.065271] [81438107] put_device+0x17/0x20
[ 1015.126794] [8143918a] device_unregister+0x2a/0x60
[ 1015.195578] [8144d55b] remove_memory_block+0xbb/0xf0
[ 1015.266434] [8144d5af] unregister_memory_section+0x1f/0x30
[ 1015.343532] [811c0a58] __remove_section+0x68/0x110
[ 1015.412318] [811c0be7] __remove_pages+0xe7/0x120
[ 1015.479021] [81653d8c] arch_remove_memory+0x2c/0x80
[ 1015.548845] [8165497b] remove_memory+0x6b/0xd0
[ 1015.613474] [813d946c]
acpi_memory_device_remove_memory+0x48/0x73
[ 1015.697834] [813d94c2] acpi_memory_device_remove+0x2b/0x44
[ 1015.774922] [813a61e4] acpi_device_remove+0x90/0xb2
[ 1015.844796] [8143c2fc] __device_release_driver+0x7c/0xf0
[ 1015.919814] [8143c47f] device_release_driver+0x2f/0x50
[ 1015.992753] [813a70dc] acpi_bus_remove+0x32/0x6d
[ 1016.059462] [813a71a8] acpi_bus_trim+0x91/0x102
[ 1016.125128] [813a72a1]
acpi_bus_hot_remove_device+0x88/0x16b
[ 1016.204295] [813a2e57] acpi_os_execute_deferred+0x27/0x34
[ 1016.280350] [81090599] process_one_work+0x219/0x680
[ 1016.350173] [81090538] ? process_one_work+0x1b8/0x680
[ 1016.422072] [813a2e30] ?
acpi_os_wait_events_complete+0x23/0x23
[ 1016.504357] [810923ce] worker_thread+0x12e/0x320
[ 1016.571064] [810922a0] ? manage_workers+0x110/0x110
[ 1016.640886] [810983a6] kthread+0xc6/0xd0
[ 1016.699290] [8167b144] kernel_thread_helper+0x4/0x10
[ 1016.770149] [81670bb0] ? retint_restore_args+0x13/0x13
[ 1016.843165] [810982e0] ? __init_kthread_worker+0x70/0x70
[ 1016.918200] [8167b140] ? gs_change+0x13/0x13
Thanks,
Yasuaki Ishimatsu
static void
unregister_memory(struct memory_block *memory)
{
BUG_ON(memory-dev.bus != memory_subsys);
/* drop the ref. we got in remove_memory_block() */
kobject_put(memory-dev.kobj);
device_unregister(memory-dev);
}
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] memory-hotplug: add memory_block_release
Hi Chen,
2012/09/28 15:04, Ni zhan Chen wrote:
On 09/28/2012 11:45 AM, Yasuaki Ishimatsu wrote:
Hi Kosaki-san,
2012/09/28 10:35, KOSAKI Motohiro wrote:
On Thu, Sep 27, 2012 at 8:24 PM, Yasuaki Ishimatsu
isimatu.yasu...@jp.fujitsu.com wrote:
Hi Chen,
2012/09/27 19:20, Ni zhan Chen wrote:
Hi Congyang,
2012/9/27 we...@cn.fujitsu.com
From: Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com
When calling remove_memory_block(), the function shows following message
at
device_release().
Device 'memory528' does not have a release() function, it is broken and
must
be fixed.
What's the difference between the patch and original implemetation?
The implementation is for removing a memory_block. So the purpose is
same as original one. But original code is bad manner. kobject_cleanup()
is called by remove_memory_block() at last. But release function for
releasing memory_block is not registered. As a result, the kernel message
is shown. IMHO, memory_block should be release by the releae function.
but your patch introduced use after free bug, if i understand correctly.
See unregister_memory() function. After your patch, kobject_put() call
release_memory_block() and kfree(). and then device_unregister() will
touch freed memory.
this patch is similiar to [RFC v9 PATCH 10/21] memory-hotplug: add
memory_block_release, they handle the same issue, can these two patches be fold
to one?
You're right. The patch is same as [RFC v9 PATCH 10/21].
The patch is a bug fix. So we separated it from memory-hotplug patch-set.
Thanks,
Yasuaki Ishimatsu
It is not correct. The kobject_put() is prepared against find_memory_block()
in remove_memory_block() since kobject-kref is incremented in it.
So release_memory_block() is called by device_unregister() correctly as follows:
[ 1014.589008] Pid: 126, comm: kworker/0:2 Not tainted
3.6.0-rc3-enable-memory-hotremove-and-root-bridge #3
[ 1014.702437] Call Trace:
[ 1014.731684] [8144d096] release_memory_block+0x16/0x30
[ 1014.803581] [81438587] device_release+0x27/0xa0
[ 1014.869312] [8133e962] kobject_cleanup+0x82/0x1b0
[ 1014.937062] [8133ea9d] kobject_release+0xd/0x10
[ 1015.002718] [8133e7ec] kobject_put+0x2c/0x60
[ 1015.065271] [81438107] put_device+0x17/0x20
[ 1015.126794] [8143918a] device_unregister+0x2a/0x60
[ 1015.195578] [8144d55b] remove_memory_block+0xbb/0xf0
[ 1015.266434] [8144d5af] unregister_memory_section+0x1f/0x30
[ 1015.343532] [811c0a58] __remove_section+0x68/0x110
[ 1015.412318] [811c0be7] __remove_pages+0xe7/0x120
[ 1015.479021] [81653d8c] arch_remove_memory+0x2c/0x80
[ 1015.548845] [8165497b] remove_memory+0x6b/0xd0
[ 1015.613474] [813d946c] acpi_memory_device_remove_memory+0x48/0x73
[ 1015.697834] [813d94c2] acpi_memory_device_remove+0x2b/0x44
[ 1015.774922] [813a61e4] acpi_device_remove+0x90/0xb2
[ 1015.844796] [8143c2fc] __device_release_driver+0x7c/0xf0
[ 1015.919814] [8143c47f] device_release_driver+0x2f/0x50
[ 1015.992753] [813a70dc] acpi_bus_remove+0x32/0x6d
[ 1016.059462] [813a71a8] acpi_bus_trim+0x91/0x102
[ 1016.125128] [813a72a1] acpi_bus_hot_remove_device+0x88/0x16b
[ 1016.204295] [813a2e57] acpi_os_execute_deferred+0x27/0x34
[ 1016.280350] [81090599] process_one_work+0x219/0x680
[ 1016.350173] [81090538] ? process_one_work+0x1b8/0x680
[ 1016.422072] [813a2e30] ? acpi_os_wait_events_complete+0x23/0x23
[ 1016.504357] [810923ce] worker_thread+0x12e/0x320
[ 1016.571064] [810922a0] ? manage_workers+0x110/0x110
[ 1016.640886] [810983a6] kthread+0xc6/0xd0
[ 1016.699290] [8167b144] kernel_thread_helper+0x4/0x10
[ 1016.770149] [81670bb0] ? retint_restore_args+0x13/0x13
[ 1016.843165] [810982e0] ? __init_kthread_worker+0x70/0x70
[ 1016.918200] [8167b140] ? gs_change+0x13/0x13
Thanks,
Yasuaki Ishimatsu
static void
unregister_memory(struct memory_block *memory)
{
BUG_ON(memory-dev.bus != memory_subsys);
/* drop the ref. we got in remove_memory_block() */
kobject_put(memory-dev.kobj);
device_unregister(memory-dev);
}
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] memory-hotplug: add memory_block_release
On 09/28/2012 11:45 AM, Yasuaki Ishimatsu wrote:
Hi Kosaki-san,
2012/09/28 10:35, KOSAKI Motohiro wrote:
On Thu, Sep 27, 2012 at 8:24 PM, Yasuaki Ishimatsu
isimatu.yasu...@jp.fujitsu.com wrote:
Hi Chen,
2012/09/27 19:20, Ni zhan Chen wrote:
Hi Congyang,
2012/9/27 we...@cn.fujitsu.com
From: Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com
When calling remove_memory_block(), the function shows following
message
at
device_release().
Device 'memory528' does not have a release() function, it is
broken and
must
be fixed.
What's the difference between the patch and original implemetation?
The implementation is for removing a memory_block. So the purpose is
same as original one. But original code is bad manner.
kobject_cleanup()
is called by remove_memory_block() at last. But release function for
releasing memory_block is not registered. As a result, the kernel
message
is shown. IMHO, memory_block should be release by the releae function.
but your patch introduced use after free bug, if i understand correctly.
See unregister_memory() function. After your patch, kobject_put() call
release_memory_block() and kfree(). and then device_unregister() will
touch freed memory.
It is not correct. The kobject_put() is prepared against
find_memory_block()
in remove_memory_block() since kobject-kref is incremented in it.
So release_memory_block() is called by device_unregister() correctly
as follows:
Another issue is memory hotplug which is not associated to this patch
report to you:
IIUC, function register_mem_sect_under_node should be renamed to
register_mem_block_under_node,
since this function is register memory block instead of memory section.
[ 1014.589008] Pid: 126, comm: kworker/0:2 Not tainted
3.6.0-rc3-enable-memory-hotremove-and-root-bridge #3
[ 1014.702437] Call Trace:
[ 1014.731684] [8144d096] release_memory_block+0x16/0x30
[ 1014.803581] [81438587] device_release+0x27/0xa0
[ 1014.869312] [8133e962] kobject_cleanup+0x82/0x1b0
[ 1014.937062] [8133ea9d] kobject_release+0xd/0x10
[ 1015.002718] [8133e7ec] kobject_put+0x2c/0x60
[ 1015.065271] [81438107] put_device+0x17/0x20
[ 1015.126794] [8143918a] device_unregister+0x2a/0x60
[ 1015.195578] [8144d55b] remove_memory_block+0xbb/0xf0
[ 1015.266434] [8144d5af] unregister_memory_section+0x1f/0x30
[ 1015.343532] [811c0a58] __remove_section+0x68/0x110
[ 1015.412318] [811c0be7] __remove_pages+0xe7/0x120
[ 1015.479021] [81653d8c] arch_remove_memory+0x2c/0x80
[ 1015.548845] [8165497b] remove_memory+0x6b/0xd0
[ 1015.613474] [813d946c]
acpi_memory_device_remove_memory+0x48/0x73
[ 1015.697834] [813d94c2] acpi_memory_device_remove+0x2b/0x44
[ 1015.774922] [813a61e4] acpi_device_remove+0x90/0xb2
[ 1015.844796] [8143c2fc] __device_release_driver+0x7c/0xf0
[ 1015.919814] [8143c47f] device_release_driver+0x2f/0x50
[ 1015.992753] [813a70dc] acpi_bus_remove+0x32/0x6d
[ 1016.059462] [813a71a8] acpi_bus_trim+0x91/0x102
[ 1016.125128] [813a72a1]
acpi_bus_hot_remove_device+0x88/0x16b
[ 1016.204295] [813a2e57] acpi_os_execute_deferred+0x27/0x34
[ 1016.280350] [81090599] process_one_work+0x219/0x680
[ 1016.350173] [81090538] ? process_one_work+0x1b8/0x680
[ 1016.422072] [813a2e30] ?
acpi_os_wait_events_complete+0x23/0x23
[ 1016.504357] [810923ce] worker_thread+0x12e/0x320
[ 1016.571064] [810922a0] ? manage_workers+0x110/0x110
[ 1016.640886] [810983a6] kthread+0xc6/0xd0
[ 1016.699290] [8167b144] kernel_thread_helper+0x4/0x10
[ 1016.770149] [81670bb0] ? retint_restore_args+0x13/0x13
[ 1016.843165] [810982e0] ? __init_kthread_worker+0x70/0x70
[ 1016.918200] [8167b140] ? gs_change+0x13/0x13
Thanks,
Yasuaki Ishimatsu
static void
unregister_memory(struct memory_block *memory)
{
BUG_ON(memory-dev.bus != memory_subsys);
/* drop the ref. we got in remove_memory_block() */
kobject_put(memory-dev.kobj);
device_unregister(memory-dev);
}
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] memory-hotplug: add memory_block_release
It is not correct. The kobject_put() is prepared against find_memory_block() in remove_memory_block() since kobject-kref is incremented in it. So release_memory_block() is called by device_unregister() correctly as follows: Ok. Looks good then. Please rewrite the description more kindly at next spin. Current one is really really bad. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] memory-hotplug: add memory_block_release
Hi Kosaki-san,
2012/09/28 10:35, KOSAKI Motohiro wrote:
On Thu, Sep 27, 2012 at 8:24 PM, Yasuaki Ishimatsu
wrote:
Hi Chen,
2012/09/27 19:20, Ni zhan Chen wrote:
Hi Congyang,
2012/9/27
From: Yasuaki Ishimatsu
When calling remove_memory_block(), the function shows following message
at
device_release().
Device 'memory528' does not have a release() function, it is broken and
must
be fixed.
What's the difference between the patch and original implemetation?
The implementation is for removing a memory_block. So the purpose is
same as original one. But original code is bad manner. kobject_cleanup()
is called by remove_memory_block() at last. But release function for
releasing memory_block is not registered. As a result, the kernel message
is shown. IMHO, memory_block should be release by the releae function.
but your patch introduced use after free bug, if i understand correctly.
See unregister_memory() function. After your patch, kobject_put() call
release_memory_block() and kfree(). and then device_unregister() will
touch freed memory.
It is not correct. The kobject_put() is prepared against find_memory_block()
in remove_memory_block() since kobject->kref is incremented in it.
So release_memory_block() is called by device_unregister() correctly as follows:
[ 1014.589008] Pid: 126, comm: kworker/0:2 Not tainted
3.6.0-rc3-enable-memory-hotremove-and-root-bridge #3
[ 1014.702437] Call Trace:
[ 1014.731684] [] release_memory_block+0x16/0x30
[ 1014.803581] [] device_release+0x27/0xa0
[ 1014.869312] [] kobject_cleanup+0x82/0x1b0
[ 1014.937062] [] kobject_release+0xd/0x10
[ 1015.002718] [] kobject_put+0x2c/0x60
[ 1015.065271] [] put_device+0x17/0x20
[ 1015.126794] [] device_unregister+0x2a/0x60
[ 1015.195578] [] remove_memory_block+0xbb/0xf0
[ 1015.266434] [] unregister_memory_section+0x1f/0x30
[ 1015.343532] [] __remove_section+0x68/0x110
[ 1015.412318] [] __remove_pages+0xe7/0x120
[ 1015.479021] [] arch_remove_memory+0x2c/0x80
[ 1015.548845] [] remove_memory+0x6b/0xd0
[ 1015.613474] [] acpi_memory_device_remove_memory+0x48/0x73
[ 1015.697834] [] acpi_memory_device_remove+0x2b/0x44
[ 1015.774922] [] acpi_device_remove+0x90/0xb2
[ 1015.844796] [] __device_release_driver+0x7c/0xf0
[ 1015.919814] [] device_release_driver+0x2f/0x50
[ 1015.992753] [] acpi_bus_remove+0x32/0x6d
[ 1016.059462] [] acpi_bus_trim+0x91/0x102
[ 1016.125128] [] acpi_bus_hot_remove_device+0x88/0x16b
[ 1016.204295] [] acpi_os_execute_deferred+0x27/0x34
[ 1016.280350] [] process_one_work+0x219/0x680
[ 1016.350173] [] ? process_one_work+0x1b8/0x680
[ 1016.422072] [] ? acpi_os_wait_events_complete+0x23/0x23
[ 1016.504357] [] worker_thread+0x12e/0x320
[ 1016.571064] [] ? manage_workers+0x110/0x110
[ 1016.640886] [] kthread+0xc6/0xd0
[ 1016.699290] [] kernel_thread_helper+0x4/0x10
[ 1016.770149] [] ? retint_restore_args+0x13/0x13
[ 1016.843165] [] ? __init_kthread_worker+0x70/0x70
[ 1016.918200] [] ? gs_change+0x13/0x13
Thanks,
Yasuaki Ishimatsu
static void
unregister_memory(struct memory_block *memory)
{
BUG_ON(memory->dev.bus != _subsys);
/* drop the ref. we got in remove_memory_block() */
kobject_put(>dev.kobj);
device_unregister(>dev);
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] memory-hotplug: add memory_block_release
On Thu, Sep 27, 2012 at 8:24 PM, Yasuaki Ishimatsu
wrote:
> Hi Chen,
>
>
> 2012/09/27 19:20, Ni zhan Chen wrote:
>>
>> Hi Congyang,
>>
>> 2012/9/27
>>
>>> From: Yasuaki Ishimatsu
>>>
>>> When calling remove_memory_block(), the function shows following message
>>> at
>>> device_release().
>>>
>>> Device 'memory528' does not have a release() function, it is broken and
>>> must
>>> be fixed.
>>>
>>
>> What's the difference between the patch and original implemetation?
>
>
> The implementation is for removing a memory_block. So the purpose is
> same as original one. But original code is bad manner. kobject_cleanup()
> is called by remove_memory_block() at last. But release function for
> releasing memory_block is not registered. As a result, the kernel message
> is shown. IMHO, memory_block should be release by the releae function.
but your patch introduced use after free bug, if i understand correctly.
See unregister_memory() function. After your patch, kobject_put() call
release_memory_block() and kfree(). and then device_unregister() will
touch freed memory.
static void
unregister_memory(struct memory_block *memory)
{
BUG_ON(memory->dev.bus != _subsys);
/* drop the ref. we got in remove_memory_block() */
kobject_put(>dev.kobj);
device_unregister(>dev);
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] memory-hotplug: add memory_block_release
Hi Chen,
2012/09/27 19:20, Ni zhan Chen wrote:
Hi Congyang,
2012/9/27
From: Yasuaki Ishimatsu
When calling remove_memory_block(), the function shows following message at
device_release().
Device 'memory528' does not have a release() function, it is broken and
must
be fixed.
What's the difference between the patch and original implemetation?
The implementation is for removing a memory_block. So the purpose is
same as original one. But original code is bad manner. kobject_cleanup()
is called by remove_memory_block() at last. But release function for
releasing memory_block is not registered. As a result, the kernel message
is shown. IMHO, memory_block should be release by the releae function.
Thanks,
Yasuaki Ishimatsu
remove_memory_block() calls kfree(mem). I think it shouled be called from
device_release(). So the patch implements memory_block_release()
CC: David Rientjes
CC: Jiang Liu
CC: Len Brown
CC: Benjamin Herrenschmidt
CC: Paul Mackerras
Cc: Minchan Kim
CC: Andrew Morton
CC: KOSAKI Motohiro
CC: Wen Congyang
Signed-off-by: Yasuaki Ishimatsu
---
drivers/base/memory.c |9 -
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index 7dda4f7..da457e5 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -70,6 +70,13 @@ void unregister_memory_isolate_notifier(struct
notifier_block *nb)
}
EXPORT_SYMBOL(unregister_memory_isolate_notifier);
+static void release_memory_block(struct device *dev)
+{
+ struct memory_block *mem = container_of(dev, struct memory_block,
dev);
+
+ kfree(mem);
+}
+
/*
* register_memory - Setup a sysfs device for a memory block
*/
@@ -80,6 +87,7 @@ int register_memory(struct memory_block *memory)
memory->dev.bus = _subsys;
memory->dev.id = memory->start_section_nr / sections_per_block;
+ memory->dev.release = release_memory_block;
error = device_register(>dev);
return error;
@@ -630,7 +638,6 @@ int remove_memory_block(unsigned long node_id, struct
mem_section *section,
mem_remove_simple_file(mem, phys_device);
mem_remove_simple_file(mem, removable);
unregister_memory(mem);
- kfree(mem);
} else
kobject_put(>dev.kobj);
--
1.7.1
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majord...@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: mailto:"d...@kvack.org;> em...@kvack.org
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] memory-hotplug: add memory_block_release
On Thu, Sep 27, 2012 at 1:45 AM, wrote:
> From: Yasuaki Ishimatsu
>
> When calling remove_memory_block(), the function shows following message at
> device_release().
>
> Device 'memory528' does not have a release() function, it is broken and must
> be fixed.
>
> remove_memory_block() calls kfree(mem). I think it shouled be called from
> device_release(). So the patch implements memory_block_release()
Why do you think so? This is terribly bad change log. it has almost
zero information.
We can't review it.
>
> CC: David Rientjes
> CC: Jiang Liu
> CC: Len Brown
> CC: Benjamin Herrenschmidt
> CC: Paul Mackerras
> Cc: Minchan Kim
> CC: Andrew Morton
> CC: KOSAKI Motohiro
> CC: Wen Congyang
> Signed-off-by: Yasuaki Ishimatsu
> ---
> drivers/base/memory.c |9 -
> 1 files changed, 8 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
> index 7dda4f7..da457e5 100644
> --- a/drivers/base/memory.c
> +++ b/drivers/base/memory.c
> @@ -70,6 +70,13 @@ void unregister_memory_isolate_notifier(struct
> notifier_block *nb)
> }
> EXPORT_SYMBOL(unregister_memory_isolate_notifier);
>
> +static void release_memory_block(struct device *dev)
> +{
> + struct memory_block *mem = container_of(dev, struct memory_block,
> dev);
> +
> + kfree(mem);
> +}
> +
> /*
> * register_memory - Setup a sysfs device for a memory block
> */
> @@ -80,6 +87,7 @@ int register_memory(struct memory_block *memory)
>
> memory->dev.bus = _subsys;
> memory->dev.id = memory->start_section_nr / sections_per_block;
> + memory->dev.release = release_memory_block;
>
> error = device_register(>dev);
> return error;
> @@ -630,7 +638,6 @@ int remove_memory_block(unsigned long node_id, struct
> mem_section *section,
> mem_remove_simple_file(mem, phys_device);
> mem_remove_simple_file(mem, removable);
> unregister_memory(mem);
> - kfree(mem);
> } else
> kobject_put(>dev.kobj);
>
> --
> 1.7.1
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majord...@kvack.org. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: mailto:"d...@kvack.org;> em...@kvack.org
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] memory-hotplug: add memory_block_release
On Thu, Sep 27, 2012 at 1:45 AM, we...@cn.fujitsu.com wrote:
From: Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com
When calling remove_memory_block(), the function shows following message at
device_release().
Device 'memory528' does not have a release() function, it is broken and must
be fixed.
remove_memory_block() calls kfree(mem). I think it shouled be called from
device_release(). So the patch implements memory_block_release()
Why do you think so? This is terribly bad change log. it has almost
zero information.
We can't review it.
CC: David Rientjes rient...@google.com
CC: Jiang Liu liu...@gmail.com
CC: Len Brown len.br...@intel.com
CC: Benjamin Herrenschmidt b...@kernel.crashing.org
CC: Paul Mackerras pau...@samba.org
Cc: Minchan Kim minchan@gmail.com
CC: Andrew Morton a...@linux-foundation.org
CC: KOSAKI Motohiro kosaki.motoh...@jp.fujitsu.com
CC: Wen Congyang we...@cn.fujitsu.com
Signed-off-by: Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com
---
drivers/base/memory.c |9 -
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index 7dda4f7..da457e5 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -70,6 +70,13 @@ void unregister_memory_isolate_notifier(struct
notifier_block *nb)
}
EXPORT_SYMBOL(unregister_memory_isolate_notifier);
+static void release_memory_block(struct device *dev)
+{
+ struct memory_block *mem = container_of(dev, struct memory_block,
dev);
+
+ kfree(mem);
+}
+
/*
* register_memory - Setup a sysfs device for a memory block
*/
@@ -80,6 +87,7 @@ int register_memory(struct memory_block *memory)
memory-dev.bus = memory_subsys;
memory-dev.id = memory-start_section_nr / sections_per_block;
+ memory-dev.release = release_memory_block;
error = device_register(memory-dev);
return error;
@@ -630,7 +638,6 @@ int remove_memory_block(unsigned long node_id, struct
mem_section *section,
mem_remove_simple_file(mem, phys_device);
mem_remove_simple_file(mem, removable);
unregister_memory(mem);
- kfree(mem);
} else
kobject_put(mem-dev.kobj);
--
1.7.1
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majord...@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: a href=mailto:d...@kvack.org; em...@kvack.org /a
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] memory-hotplug: add memory_block_release
Hi Chen,
2012/09/27 19:20, Ni zhan Chen wrote:
Hi Congyang,
2012/9/27 we...@cn.fujitsu.com
From: Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com
When calling remove_memory_block(), the function shows following message at
device_release().
Device 'memory528' does not have a release() function, it is broken and
must
be fixed.
What's the difference between the patch and original implemetation?
The implementation is for removing a memory_block. So the purpose is
same as original one. But original code is bad manner. kobject_cleanup()
is called by remove_memory_block() at last. But release function for
releasing memory_block is not registered. As a result, the kernel message
is shown. IMHO, memory_block should be release by the releae function.
Thanks,
Yasuaki Ishimatsu
remove_memory_block() calls kfree(mem). I think it shouled be called from
device_release(). So the patch implements memory_block_release()
CC: David Rientjes rient...@google.com
CC: Jiang Liu liu...@gmail.com
CC: Len Brown len.br...@intel.com
CC: Benjamin Herrenschmidt b...@kernel.crashing.org
CC: Paul Mackerras pau...@samba.org
Cc: Minchan Kim minchan@gmail.com
CC: Andrew Morton a...@linux-foundation.org
CC: KOSAKI Motohiro kosaki.motoh...@jp.fujitsu.com
CC: Wen Congyang we...@cn.fujitsu.com
Signed-off-by: Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com
---
drivers/base/memory.c |9 -
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index 7dda4f7..da457e5 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -70,6 +70,13 @@ void unregister_memory_isolate_notifier(struct
notifier_block *nb)
}
EXPORT_SYMBOL(unregister_memory_isolate_notifier);
+static void release_memory_block(struct device *dev)
+{
+ struct memory_block *mem = container_of(dev, struct memory_block,
dev);
+
+ kfree(mem);
+}
+
/*
* register_memory - Setup a sysfs device for a memory block
*/
@@ -80,6 +87,7 @@ int register_memory(struct memory_block *memory)
memory-dev.bus = memory_subsys;
memory-dev.id = memory-start_section_nr / sections_per_block;
+ memory-dev.release = release_memory_block;
error = device_register(memory-dev);
return error;
@@ -630,7 +638,6 @@ int remove_memory_block(unsigned long node_id, struct
mem_section *section,
mem_remove_simple_file(mem, phys_device);
mem_remove_simple_file(mem, removable);
unregister_memory(mem);
- kfree(mem);
} else
kobject_put(mem-dev.kobj);
--
1.7.1
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majord...@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: a href=mailto:d...@kvack.org; em...@kvack.org /a
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] memory-hotplug: add memory_block_release
On Thu, Sep 27, 2012 at 8:24 PM, Yasuaki Ishimatsu
isimatu.yasu...@jp.fujitsu.com wrote:
Hi Chen,
2012/09/27 19:20, Ni zhan Chen wrote:
Hi Congyang,
2012/9/27 we...@cn.fujitsu.com
From: Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com
When calling remove_memory_block(), the function shows following message
at
device_release().
Device 'memory528' does not have a release() function, it is broken and
must
be fixed.
What's the difference between the patch and original implemetation?
The implementation is for removing a memory_block. So the purpose is
same as original one. But original code is bad manner. kobject_cleanup()
is called by remove_memory_block() at last. But release function for
releasing memory_block is not registered. As a result, the kernel message
is shown. IMHO, memory_block should be release by the releae function.
but your patch introduced use after free bug, if i understand correctly.
See unregister_memory() function. After your patch, kobject_put() call
release_memory_block() and kfree(). and then device_unregister() will
touch freed memory.
static void
unregister_memory(struct memory_block *memory)
{
BUG_ON(memory-dev.bus != memory_subsys);
/* drop the ref. we got in remove_memory_block() */
kobject_put(memory-dev.kobj);
device_unregister(memory-dev);
}
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] memory-hotplug: add memory_block_release
Hi Kosaki-san,
2012/09/28 10:35, KOSAKI Motohiro wrote:
On Thu, Sep 27, 2012 at 8:24 PM, Yasuaki Ishimatsu
isimatu.yasu...@jp.fujitsu.com wrote:
Hi Chen,
2012/09/27 19:20, Ni zhan Chen wrote:
Hi Congyang,
2012/9/27 we...@cn.fujitsu.com
From: Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com
When calling remove_memory_block(), the function shows following message
at
device_release().
Device 'memory528' does not have a release() function, it is broken and
must
be fixed.
What's the difference between the patch and original implemetation?
The implementation is for removing a memory_block. So the purpose is
same as original one. But original code is bad manner. kobject_cleanup()
is called by remove_memory_block() at last. But release function for
releasing memory_block is not registered. As a result, the kernel message
is shown. IMHO, memory_block should be release by the releae function.
but your patch introduced use after free bug, if i understand correctly.
See unregister_memory() function. After your patch, kobject_put() call
release_memory_block() and kfree(). and then device_unregister() will
touch freed memory.
It is not correct. The kobject_put() is prepared against find_memory_block()
in remove_memory_block() since kobject-kref is incremented in it.
So release_memory_block() is called by device_unregister() correctly as follows:
[ 1014.589008] Pid: 126, comm: kworker/0:2 Not tainted
3.6.0-rc3-enable-memory-hotremove-and-root-bridge #3
[ 1014.702437] Call Trace:
[ 1014.731684] [8144d096] release_memory_block+0x16/0x30
[ 1014.803581] [81438587] device_release+0x27/0xa0
[ 1014.869312] [8133e962] kobject_cleanup+0x82/0x1b0
[ 1014.937062] [8133ea9d] kobject_release+0xd/0x10
[ 1015.002718] [8133e7ec] kobject_put+0x2c/0x60
[ 1015.065271] [81438107] put_device+0x17/0x20
[ 1015.126794] [8143918a] device_unregister+0x2a/0x60
[ 1015.195578] [8144d55b] remove_memory_block+0xbb/0xf0
[ 1015.266434] [8144d5af] unregister_memory_section+0x1f/0x30
[ 1015.343532] [811c0a58] __remove_section+0x68/0x110
[ 1015.412318] [811c0be7] __remove_pages+0xe7/0x120
[ 1015.479021] [81653d8c] arch_remove_memory+0x2c/0x80
[ 1015.548845] [8165497b] remove_memory+0x6b/0xd0
[ 1015.613474] [813d946c] acpi_memory_device_remove_memory+0x48/0x73
[ 1015.697834] [813d94c2] acpi_memory_device_remove+0x2b/0x44
[ 1015.774922] [813a61e4] acpi_device_remove+0x90/0xb2
[ 1015.844796] [8143c2fc] __device_release_driver+0x7c/0xf0
[ 1015.919814] [8143c47f] device_release_driver+0x2f/0x50
[ 1015.992753] [813a70dc] acpi_bus_remove+0x32/0x6d
[ 1016.059462] [813a71a8] acpi_bus_trim+0x91/0x102
[ 1016.125128] [813a72a1] acpi_bus_hot_remove_device+0x88/0x16b
[ 1016.204295] [813a2e57] acpi_os_execute_deferred+0x27/0x34
[ 1016.280350] [81090599] process_one_work+0x219/0x680
[ 1016.350173] [81090538] ? process_one_work+0x1b8/0x680
[ 1016.422072] [813a2e30] ? acpi_os_wait_events_complete+0x23/0x23
[ 1016.504357] [810923ce] worker_thread+0x12e/0x320
[ 1016.571064] [810922a0] ? manage_workers+0x110/0x110
[ 1016.640886] [810983a6] kthread+0xc6/0xd0
[ 1016.699290] [8167b144] kernel_thread_helper+0x4/0x10
[ 1016.770149] [81670bb0] ? retint_restore_args+0x13/0x13
[ 1016.843165] [810982e0] ? __init_kthread_worker+0x70/0x70
[ 1016.918200] [8167b140] ? gs_change+0x13/0x13
Thanks,
Yasuaki Ishimatsu
static void
unregister_memory(struct memory_block *memory)
{
BUG_ON(memory-dev.bus != memory_subsys);
/* drop the ref. we got in remove_memory_block() */
kobject_put(memory-dev.kobj);
device_unregister(memory-dev);
}
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
