linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Vlastimil Babka <vbabka@suse.cz>
To: Christoph Lameter <cl@linux.com>,
	Pekka Enberg <penberg@kernel.org>,
	David Rientjes <rientjes@google.com>,
	Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	kernel-team@android.com, vinmenon@codeaurora.org,
	Kees Cook <keescook@chromium.org>,
	Matthew Garrett <mjg59@google.com>,
	Vlastimil Babka <vbabka@suse.cz>, Jann Horn <jannh@google.com>,
	Vijayanand Jitta <vjitta@codeaurora.org>
Subject: [RFC PATCH 3/5] mm, slub: remove runtime allocation order changes
Date: Tue,  2 Jun 2020 16:15:17 +0200	[thread overview]
Message-ID: <20200602141519.7099-4-vbabka@suse.cz> (raw)
In-Reply-To: <20200602141519.7099-1-vbabka@suse.cz>

SLUB allows runtime changing of page allocation order by writing into the
/sys/kernel/slab/<cache>/order file. Jann has reported [1] that this interface
allows the order to be set too small, leading to crashes.

While it's possible to fix the immediate issue, closer inspection reveals
potential races. Storing the new order calls calculate_sizes() which
non-atomically updates a lot of kmem_cache fields while the cache is still in
use. Unexpected behavior might occur even if the fields are set to the same
value as they were.

This could be fixed by splitting out the part of calculate_sizes() that depends
on forced_order, so that we only update kmem_cache.oo field. This could still
race with init_cache_random_seq(), shuffle_freelist(), allocate_slab(). Perhaps
it's possible to audit and e.g. add some READ_ONCE/WRITE_ONCE accesses, it
might be easier just to remove the runtime order changes, which is what this
patch does. If there are valid usecases for per-cache order setting, we could
e.g. extend the boot parameters to do that.

[1] https://lore.kernel.org/r/CAG48ez31PP--h6_FzVyfJ4H86QYczAFPdxtJHUEEan+7VJETAQ@mail.gmail.com

Reported-by: Jann Horn <jannh@google.com>
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
 mm/slub.c | 19 +------------------
 1 file changed, 1 insertion(+), 18 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index ac198202dbb0..58c1e9e7b3b3 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -5111,28 +5111,11 @@ static ssize_t objs_per_slab_show(struct kmem_cache *s, char *buf)
 }
 SLAB_ATTR_RO(objs_per_slab);
 
-static ssize_t order_store(struct kmem_cache *s,
-				const char *buf, size_t length)
-{
-	unsigned int order;
-	int err;
-
-	err = kstrtouint(buf, 10, &order);
-	if (err)
-		return err;
-
-	if (order > slub_max_order || order < slub_min_order)
-		return -EINVAL;
-
-	calculate_sizes(s, order);
-	return length;
-}
-
 static ssize_t order_show(struct kmem_cache *s, char *buf)
 {
 	return sprintf(buf, "%u\n", oo_order(s->oo));
 }
-SLAB_ATTR(order);
+SLAB_ATTR_RO(order);
 
 static ssize_t min_partial_show(struct kmem_cache *s, char *buf)
 {
-- 
2.26.2



  parent reply	other threads:[~2020-06-02 14:16 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-02 14:15 [RFC PATCH 0/5] replace runtime slub_debug toggling with more capable boot parameter Vlastimil Babka
2020-06-02 14:15 ` [RFC PATCH 1/5] mm, slub: extend slub_debug syntax for multiple blocks Vlastimil Babka
2020-06-05 21:06   ` Kees Cook
2020-06-08 16:58     ` Vlastimil Babka
2020-06-02 14:15 ` [RFC PATCH 2/5] mm, slub: make some slub_debug related attributes read-only Vlastimil Babka
2020-06-05 21:06   ` Kees Cook
2020-06-06  0:32   ` Roman Gushchin
2020-06-02 14:15 ` Vlastimil Babka [this message]
2020-06-05 21:06   ` [RFC PATCH 3/5] mm, slub: remove runtime allocation order changes Kees Cook
2020-06-06  0:32   ` Roman Gushchin
2020-06-02 14:15 ` [RFC PATCH 4/5] mm, slub: make remaining slub_debug related attributes read-only Vlastimil Babka
2020-06-05 21:07   ` Kees Cook
2020-06-06  0:33   ` Roman Gushchin
2020-06-02 14:15 ` [RFC PATCH 5/5] mm, slub: make reclaim_account attribute read-only Vlastimil Babka
2020-06-05 21:07   ` Kees Cook
2020-06-06  0:34   ` Roman Gushchin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200602141519.7099-4-vbabka@suse.cz \
    --to=vbabka@suse.cz \
    --cc=cl@linux.com \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=jannh@google.com \
    --cc=keescook@chromium.org \
    --cc=kernel-team@android.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mjg59@google.com \
    --cc=penberg@kernel.org \
    --cc=rientjes@google.com \
    --cc=vinmenon@codeaurora.org \
    --cc=vjitta@codeaurora.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox