From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751469AbdETKwI (ORCPT ); Sat, 20 May 2017 06:52:08 -0400 Received: from Galois.linutronix.de ([146.0.238.70]:60436 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750735AbdETKwH (ORCPT ); Sat, 20 May 2017 06:52:07 -0400 Date: Sat, 20 May 2017 12:52:03 +0200 (CEST) From: Thomas Gleixner To: LKML cc: linux-mm@kvack.org, Johannes Weiner , Michal Hocko , Christoph Lameter , Andrew Morton , Steven Rostedt , Peter Zijlstra Subject: [PATCH] slub/memcg: Cure the brainless abuse of sysfs attributes Message-ID: User-Agent: Alpine 2.20 (DEB 67 2015-01-07) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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. Reported-by: Steven Rostedt Signed-off-by: Thomas Gleixner Cc: stable@vger.kernel.org --- mm/slub.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) --- a/mm/slub.c +++ b/mm/slub.c @@ -5512,6 +5512,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; @@ -5536,8 +5537,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) From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f70.google.com (mail-wm0-f70.google.com [74.125.82.70]) by kanga.kvack.org (Postfix) with ESMTP id A8D47280753 for ; Sat, 20 May 2017 06:52:07 -0400 (EDT) Received: by mail-wm0-f70.google.com with SMTP id b84so17313545wmh.0 for ; Sat, 20 May 2017 03:52:07 -0700 (PDT) Received: from Galois.linutronix.de (Galois.linutronix.de. [2a01:7a0:2:106d:700::1]) by mx.google.com with ESMTPS id p21si16722820wmf.44.2017.05.20.03.52.05 for (version=TLS1_2 cipher=AES128-SHA bits=128/128); Sat, 20 May 2017 03:52:06 -0700 (PDT) Date: Sat, 20 May 2017 12:52:03 +0200 (CEST) From: Thomas Gleixner Subject: [PATCH] slub/memcg: Cure the brainless abuse of sysfs attributes Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Sender: owner-linux-mm@kvack.org List-ID: To: LKML Cc: linux-mm@kvack.org, Johannes Weiner , Michal Hocko , Christoph Lameter , Andrew Morton , Steven Rostedt , Peter Zijlstra 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. Reported-by: Steven Rostedt Signed-off-by: Thomas Gleixner Cc: stable@vger.kernel.org --- mm/slub.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) --- a/mm/slub.c +++ b/mm/slub.c @@ -5512,6 +5512,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; @@ -5536,8 +5537,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) -- 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: email@kvack.org