* [PATCH -mm] slub: kmem_cache_shrink: init discard list after freeing slabs [not found] <1423627463.5968.99.camel@intel.com> @ 2015-02-11 8:16 ` Vladimir Davydov 2015-02-11 14:51 ` Christoph Lameter 0 siblings, 1 reply; 6+ messages in thread From: Vladimir Davydov @ 2015-02-11 8:16 UTC (permalink / raw) To: Andrew Morton Cc: linux-mm, linux-kernel, Huang Ying, Christoph Lameter, Pekka Enberg, David Rientjes, Joonsoo Kim Otherwise, if there are > 1 nodes, we can get use-after-free while processing the second or higher node: WARNING: CPU: 60 PID: 1 at lib/list_debug.c:29 __list_add+0x3c/0xa9() list_add corruption. next->prev should be prev (ffff881ff0a6bb98), but was ffffea007ff57020. (next=ffffea007fbf7320). Modules linked in: CPU: 60 PID: 1 Comm: swapper/0 Not tainted 3.19.0-rc7-next-20150203-gb50cadf #2178 Hardware name: Intel Corporation BRICKLAND/BRICKLAND, BIOS BIVTSDP1.86B.0038.R02.1307231126 07/23/2013 0000000000000009 ffff881ff0a6ba88 ffffffff81c2e096 ffffffff810e2d03 ffff881ff0a6bad8 ffff881ff0a6bac8 ffffffff8108b320 ffff881ff0a6bb18 ffffffff8154bbc7 ffff881ff0a6bb98 ffffea007fbf7320 ffffea00ffc3c220 Call Trace: [<ffffffff81c2e096>] dump_stack+0x4c/0x65 [<ffffffff810e2d03>] ? console_unlock+0x398/0x3c7 [<ffffffff8108b320>] warn_slowpath_common+0xa1/0xbb [<ffffffff8154bbc7>] ? __list_add+0x3c/0xa9 [<ffffffff8108b380>] warn_slowpath_fmt+0x46/0x48 [<ffffffff8154bbc7>] __list_add+0x3c/0xa9 [<ffffffff811bf5aa>] __kmem_cache_shrink+0x12b/0x24c [<ffffffff81190ca9>] kmem_cache_shrink+0x26/0x38 [<ffffffff815848b4>] acpi_os_purge_cache+0xe/0x12 [<ffffffff815c6424>] acpi_purge_cached_objects+0x32/0x7a [<ffffffff825f70f1>] acpi_initialize_objects+0x17e/0x1ae [<ffffffff825f5177>] ? acpi_sleep_proc_init+0x2a/0x2a [<ffffffff825f5209>] acpi_init+0x92/0x25e [<ffffffff810002bd>] ? do_one_initcall+0x90/0x17f [<ffffffff811bdfcd>] ? kfree+0x1fc/0x2d5 [<ffffffff825f5177>] ? acpi_sleep_proc_init+0x2a/0x2a [<ffffffff8100031a>] do_one_initcall+0xed/0x17f [<ffffffff825ae0e2>] kernel_init_freeable+0x1f0/0x278 [<ffffffff81c1f31a>] ? rest_init+0x13e/0x13e [<ffffffff81c1f328>] kernel_init+0xe/0xda [<ffffffff81c3ca7c>] ret_from_fork+0x7c/0xb0 [<ffffffff81c1f31a>] ? rest_init+0x13e/0x13e fixes: slub-never-fail-to-shrink-cache Signed-off-by: Vladimir Davydov <vdavydov@parallels.com> Reported-by: Huang Ying <ying.huang@intel.com> Cc: Christoph Lameter <cl@linux.com> Cc: Pekka Enberg <penberg@kernel.org> Cc: David Rientjes <rientjes@google.com> Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com> --- mm/slub.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mm/slub.c b/mm/slub.c index 0909e13cf708..59dde3f3efed 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -3499,6 +3499,8 @@ int __kmem_cache_shrink(struct kmem_cache *s, bool deactivate) list_for_each_entry_safe(page, t, &discard, lru) discard_slab(s, page); + INIT_LIST_HEAD(&discard); + if (slabs_node(s, node)) ret = 1; } -- 1.7.10.4 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH -mm] slub: kmem_cache_shrink: init discard list after freeing slabs 2015-02-11 8:16 ` [PATCH -mm] slub: kmem_cache_shrink: init discard list after freeing slabs Vladimir Davydov @ 2015-02-11 14:51 ` Christoph Lameter 2015-02-11 15:00 ` Christoph Lameter 0 siblings, 1 reply; 6+ messages in thread From: Christoph Lameter @ 2015-02-11 14:51 UTC (permalink / raw) To: Vladimir Davydov Cc: Andrew Morton, linux-mm, linux-kernel, Huang Ying, Pekka Enberg, David Rientjes, Joonsoo Kim On Wed, 11 Feb 2015, Vladimir Davydov wrote: > Otherwise, if there are > 1 nodes, we can get use-after-free while > processing the second or higher node: Acked-by: Christoph Lameter <cl@linux.com> -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH -mm] slub: kmem_cache_shrink: init discard list after freeing slabs 2015-02-11 14:51 ` Christoph Lameter @ 2015-02-11 15:00 ` Christoph Lameter 2015-02-11 15:41 ` Vladimir Davydov 0 siblings, 1 reply; 6+ messages in thread From: Christoph Lameter @ 2015-02-11 15:00 UTC (permalink / raw) To: Vladimir Davydov Cc: Andrew Morton, linux-mm, linux-kernel, Huang Ying, Pekka Enberg, David Rientjes, Joonsoo Kim Hmmmm... Thinking about this some more. It may be better to initialize the list head at the beginning of the loop? Also the promote array should also be initialized in the loop right? -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH -mm] slub: kmem_cache_shrink: init discard list after freeing slabs 2015-02-11 15:00 ` Christoph Lameter @ 2015-02-11 15:41 ` Vladimir Davydov 2015-02-11 15:44 ` Christoph Lameter 2015-02-11 20:09 ` David Rientjes 0 siblings, 2 replies; 6+ messages in thread From: Vladimir Davydov @ 2015-02-11 15:41 UTC (permalink / raw) To: Christoph Lameter Cc: Andrew Morton, linux-mm, linux-kernel, Huang Ying, Pekka Enberg, David Rientjes, Joonsoo Kim On Wed, Feb 11, 2015 at 09:00:39AM -0600, Christoph Lameter wrote: > Hmmmm... Thinking about this some more. It may be better to initialize the > list head at the beginning of the loop? > > Also the promote array should also be initialized in the loop right? I do initialize promote lists in the loop using list_splice_init, but yeah, initializing them in the beginning of the loop would look more readable indeed. The updated patch is below. Thanks! --- From: Vladimir Davydov <vdavydov@parallels.com> Subject: [PATCH] slub: kmem_cache_shrink: fix crash due to uninitialized discard list Currently, the discard list is only initialized at the beginning of the function. As a result, if there are > 1 nodes, we can get use-after-free while processing the second or higher node: WARNING: CPU: 60 PID: 1 at lib/list_debug.c:29 __list_add+0x3c/0xa9() list_add corruption. next->prev should be prev (ffff881ff0a6bb98), but was ffffea007ff57020. (next=ffffea007fbf7320). Modules linked in: CPU: 60 PID: 1 Comm: swapper/0 Not tainted 3.19.0-rc7-next-20150203-gb50cadf #2178 Hardware name: Intel Corporation BRICKLAND/BRICKLAND, BIOS BIVTSDP1.86B.0038.R02.1307231126 07/23/2013 0000000000000009 ffff881ff0a6ba88 ffffffff81c2e096 ffffffff810e2d03 ffff881ff0a6bad8 ffff881ff0a6bac8 ffffffff8108b320 ffff881ff0a6bb18 ffffffff8154bbc7 ffff881ff0a6bb98 ffffea007fbf7320 ffffea00ffc3c220 Call Trace: [<ffffffff81c2e096>] dump_stack+0x4c/0x65 [<ffffffff810e2d03>] ? console_unlock+0x398/0x3c7 [<ffffffff8108b320>] warn_slowpath_common+0xa1/0xbb [<ffffffff8154bbc7>] ? __list_add+0x3c/0xa9 [<ffffffff8108b380>] warn_slowpath_fmt+0x46/0x48 [<ffffffff8154bbc7>] __list_add+0x3c/0xa9 [<ffffffff811bf5aa>] __kmem_cache_shrink+0x12b/0x24c [<ffffffff81190ca9>] kmem_cache_shrink+0x26/0x38 [<ffffffff815848b4>] acpi_os_purge_cache+0xe/0x12 [<ffffffff815c6424>] acpi_purge_cached_objects+0x32/0x7a [<ffffffff825f70f1>] acpi_initialize_objects+0x17e/0x1ae [<ffffffff825f5177>] ? acpi_sleep_proc_init+0x2a/0x2a [<ffffffff825f5209>] acpi_init+0x92/0x25e [<ffffffff810002bd>] ? do_one_initcall+0x90/0x17f [<ffffffff811bdfcd>] ? kfree+0x1fc/0x2d5 [<ffffffff825f5177>] ? acpi_sleep_proc_init+0x2a/0x2a [<ffffffff8100031a>] do_one_initcall+0xed/0x17f [<ffffffff825ae0e2>] kernel_init_freeable+0x1f0/0x278 [<ffffffff81c1f31a>] ? rest_init+0x13e/0x13e [<ffffffff81c1f328>] kernel_init+0xe/0xda [<ffffffff81c3ca7c>] ret_from_fork+0x7c/0xb0 [<ffffffff81c1f31a>] ? rest_init+0x13e/0x13e Fix this by initializing the discard list at each iteration of the for_each_kmem_cache_node loop. Also, move promote lists initialization to the beginning of the loop to conform. fixes: slub-never-fail-to-shrink-cache Signed-off-by: Vladimir Davydov <vdavydov@parallels.com> Reported-by: Huang Ying <ying.huang@intel.com> Cc: Christoph Lameter <cl@linux.com> Cc: Pekka Enberg <penberg@kernel.org> Cc: David Rientjes <rientjes@google.com> Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com> diff --git a/mm/slub.c b/mm/slub.c index 0909e13cf708..6832c4eab104 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -3437,7 +3437,7 @@ int __kmem_cache_shrink(struct kmem_cache *s, bool deactivate) struct kmem_cache_node *n; struct page *page; struct page *t; - LIST_HEAD(discard); + struct list_head discard; struct list_head promote[SHRINK_PROMOTE_MAX]; unsigned long flags; int ret = 0; @@ -3457,11 +3457,12 @@ int __kmem_cache_shrink(struct kmem_cache *s, bool deactivate) kick_all_cpus_sync(); } - for (i = 0; i < SHRINK_PROMOTE_MAX; i++) - INIT_LIST_HEAD(promote + i); - flush_all(s); for_each_kmem_cache_node(s, node, n) { + INIT_LIST_HEAD(&discard); + for (i = 0; i < SHRINK_PROMOTE_MAX; i++) + INIT_LIST_HEAD(promote + i); + spin_lock_irqsave(&n->list_lock, flags); /* @@ -3491,7 +3492,7 @@ int __kmem_cache_shrink(struct kmem_cache *s, bool deactivate) * partial list. */ for (i = SHRINK_PROMOTE_MAX - 1; i >= 0; i--) - list_splice_init(promote + i, &n->partial); + list_splice(promote + i, &n->partial); spin_unlock_irqrestore(&n->list_lock, flags); -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH -mm] slub: kmem_cache_shrink: init discard list after freeing slabs 2015-02-11 15:41 ` Vladimir Davydov @ 2015-02-11 15:44 ` Christoph Lameter 2015-02-11 20:09 ` David Rientjes 1 sibling, 0 replies; 6+ messages in thread From: Christoph Lameter @ 2015-02-11 15:44 UTC (permalink / raw) To: Vladimir Davydov Cc: Andrew Morton, linux-mm, linux-kernel, Huang Ying, Pekka Enberg, David Rientjes, Joonsoo Kim On Wed, 11 Feb 2015, Vladimir Davydov wrote: > > Also the promote array should also be initialized in the loop right? > > I do initialize promote lists in the loop using list_splice_init, but > yeah, initializing them in the beginning of the loop would look more > readable indeed. The updated patch is below. Thanks! Acked-by: Christoph Lameter <cl@linux.com> -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH -mm] slub: kmem_cache_shrink: init discard list after freeing slabs 2015-02-11 15:41 ` Vladimir Davydov 2015-02-11 15:44 ` Christoph Lameter @ 2015-02-11 20:09 ` David Rientjes 1 sibling, 0 replies; 6+ messages in thread From: David Rientjes @ 2015-02-11 20:09 UTC (permalink / raw) To: Vladimir Davydov Cc: Christoph Lameter, Andrew Morton, linux-mm, linux-kernel, Huang Ying, Pekka Enberg, Joonsoo Kim On Wed, 11 Feb 2015, Vladimir Davydov wrote: > Currently, the discard list is only initialized at the beginning of the > function. As a result, if there are > 1 nodes, we can get use-after-free > while processing the second or higher node: > > WARNING: CPU: 60 PID: 1 at lib/list_debug.c:29 __list_add+0x3c/0xa9() > list_add corruption. next->prev should be prev (ffff881ff0a6bb98), but was ffffea007ff57020. (next=ffffea007fbf7320). > Modules linked in: > CPU: 60 PID: 1 Comm: swapper/0 Not tainted 3.19.0-rc7-next-20150203-gb50cadf #2178 > Hardware name: Intel Corporation BRICKLAND/BRICKLAND, BIOS BIVTSDP1.86B.0038.R02.1307231126 07/23/2013 > 0000000000000009 ffff881ff0a6ba88 ffffffff81c2e096 ffffffff810e2d03 > ffff881ff0a6bad8 ffff881ff0a6bac8 ffffffff8108b320 ffff881ff0a6bb18 > ffffffff8154bbc7 ffff881ff0a6bb98 ffffea007fbf7320 ffffea00ffc3c220 > Call Trace: > [<ffffffff81c2e096>] dump_stack+0x4c/0x65 > [<ffffffff810e2d03>] ? console_unlock+0x398/0x3c7 > [<ffffffff8108b320>] warn_slowpath_common+0xa1/0xbb > [<ffffffff8154bbc7>] ? __list_add+0x3c/0xa9 > [<ffffffff8108b380>] warn_slowpath_fmt+0x46/0x48 > [<ffffffff8154bbc7>] __list_add+0x3c/0xa9 > [<ffffffff811bf5aa>] __kmem_cache_shrink+0x12b/0x24c > [<ffffffff81190ca9>] kmem_cache_shrink+0x26/0x38 > [<ffffffff815848b4>] acpi_os_purge_cache+0xe/0x12 > [<ffffffff815c6424>] acpi_purge_cached_objects+0x32/0x7a > [<ffffffff825f70f1>] acpi_initialize_objects+0x17e/0x1ae > [<ffffffff825f5177>] ? acpi_sleep_proc_init+0x2a/0x2a > [<ffffffff825f5209>] acpi_init+0x92/0x25e > [<ffffffff810002bd>] ? do_one_initcall+0x90/0x17f > [<ffffffff811bdfcd>] ? kfree+0x1fc/0x2d5 > [<ffffffff825f5177>] ? acpi_sleep_proc_init+0x2a/0x2a > [<ffffffff8100031a>] do_one_initcall+0xed/0x17f > [<ffffffff825ae0e2>] kernel_init_freeable+0x1f0/0x278 > [<ffffffff81c1f31a>] ? rest_init+0x13e/0x13e > [<ffffffff81c1f328>] kernel_init+0xe/0xda > [<ffffffff81c3ca7c>] ret_from_fork+0x7c/0xb0 > [<ffffffff81c1f31a>] ? rest_init+0x13e/0x13e > > Fix this by initializing the discard list at each iteration of the > for_each_kmem_cache_node loop. Also, move promote lists initialization > to the beginning of the loop to conform. > > fixes: slub-never-fail-to-shrink-cache 8f44a586ac86 ("slub: never fail to shrink cache") > Signed-off-by: Vladimir Davydov <vdavydov@parallels.com> > Reported-by: Huang Ying <ying.huang@intel.com> > Cc: Christoph Lameter <cl@linux.com> > Cc: Pekka Enberg <penberg@kernel.org> > Cc: David Rientjes <rientjes@google.com> > Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com> Acked-by: David Rientjes <rientjes@google.com> -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-02-11 20:09 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1423627463.5968.99.camel@intel.com>
2015-02-11 8:16 ` [PATCH -mm] slub: kmem_cache_shrink: init discard list after freeing slabs Vladimir Davydov
2015-02-11 14:51 ` Christoph Lameter
2015-02-11 15:00 ` Christoph Lameter
2015-02-11 15:41 ` Vladimir Davydov
2015-02-11 15:44 ` Christoph Lameter
2015-02-11 20:09 ` David Rientjes
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).