| From 478fe3037b2278d276d4cd9cd0ab06c4cb2e9b32 Mon Sep 17 00:00:00 2001 |
| From: Thomas Gleixner <tglx@linutronix.de> |
| Date: Fri, 2 Jun 2017 14:46:25 -0700 |
| Subject: slub/memcg: cure the brainless abuse of sysfs attributes |
| |
| From: Thomas Gleixner <tglx@linutronix.de> |
| |
| commit 478fe3037b2278d276d4cd9cd0ab06c4cb2e9b32 upstream. |
| |
| memcg_propagate_slab_attrs() abuses the sysfs attribute file functions |
| to propagate settings from the root kmem_cache to a newly created |
| kmem_cache. It does that with: |
| |
| attr->show(root, buf); |
| attr->store(new, buf, strlen(bug); |
| |
| Aside of being a lazy and absurd hackery this is broken because it does |
| not check the return value of the show() function. |
| |
| Some of the show() functions return 0 w/o touching the buffer. That |
| means in such a case the store function is called with the stale content |
| of the previous show(). That causes nonsense like invoking |
| kmem_cache_shrink() on a newly created kmem_cache. In the worst case it |
| would cause handing in an uninitialized buffer. |
| |
| This should be rewritten proper by adding a propagate() callback to |
| those slub_attributes which must be propagated and avoid that insane |
| conversion to and from ASCII, but that's too large for a hot fix. |
| |
| Check at least the return value of the show() function, so calling |
| store() with stale content is prevented. |
| |
| Steven said: |
| "It can cause a deadlock with get_online_cpus() that has been uncovered |
| by recent cpu hotplug and lockdep changes that Thomas and Peter have |
| been doing. |
| |
| Possible unsafe locking scenario: |
| |
| CPU0 CPU1 |
| ---- ---- |
| lock(cpu_hotplug.lock); |
| lock(slab_mutex); |
| lock(cpu_hotplug.lock); |
| lock(slab_mutex); |
| |
| *** DEADLOCK ***" |
| |
| Link: http://lkml.kernel.org/r/alpine.DEB.2.20.1705201244540.2255@nanos |
| Signed-off-by: Thomas Gleixner <tglx@linutronix.de> |
| Reported-by: Steven Rostedt <rostedt@goodmis.org> |
| Acked-by: David Rientjes <rientjes@google.com> |
| Cc: Johannes Weiner <hannes@cmpxchg.org> |
| Cc: Michal Hocko <mhocko@kernel.org> |
| Cc: Peter Zijlstra <peterz@infradead.org> |
| Cc: Christoph Lameter <cl@linux.com> |
| Cc: Pekka Enberg <penberg@kernel.org> |
| Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com> |
| Cc: Christoph Hellwig <hch@infradead.org> |
| Signed-off-by: Andrew Morton <akpm@linux-foundation.org> |
| Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| mm/slub.c | 6 ++++-- |
| 1 file changed, 4 insertions(+), 2 deletions(-) |
| |
| --- a/mm/slub.c |
| +++ b/mm/slub.c |
| @@ -5452,6 +5452,7 @@ static void memcg_propagate_slab_attrs(s |
| char mbuf[64]; |
| char *buf; |
| struct slab_attribute *attr = to_slab_attr(slab_attrs[i]); |
| + ssize_t len; |
| |
| if (!attr || !attr->store || !attr->show) |
| continue; |
| @@ -5476,8 +5477,9 @@ static void memcg_propagate_slab_attrs(s |
| buf = buffer; |
| } |
| |
| - attr->show(root_cache, buf); |
| - attr->store(s, buf, strlen(buf)); |
| + len = attr->show(root_cache, buf); |
| + if (len > 0) |
| + attr->store(s, buf, len); |
| } |
| |
| if (buffer) |