From: Nicolas Bouchinet <nicolas.bouchinet@clip-os.org>
To: linux-mm@kvack.org, linux-kernel@vger.kernel.org
Cc: Chengming Zhou <chengming.zhou@linux.dev>,
Christoph Lameter <cl@linux.com>,
Pekka Enberg <penberg@kernel.org>,
David Rientjes <rientjes@google.com>,
Joonsoo Kim <iamjoonsoo.kim@lge.com>,
Andrew Morton <akpm@linux-foundation.org>,
Vlastimil Babka <vbabka@suse.cz>,
Roman Gushchin <roman.gushchin@linux.dev>,
Hyeonggon Yoo <42.hyeyoo@gmail.com>
Subject: [PATCH v3] slub: Fixes freepointer encoding for single free
Date: Tue, 30 Apr 2024 13:34:59 +0200 [thread overview]
Message-ID: <ZjDXY74yS6UyQPxv@archlinux> (raw)
From: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
Commit 284f17ac13fe ("mm/slub: handle bulk and single object freeing
separately") splits single and bulk object freeing in two functions
slab_free() and slab_free_bulk() which leads slab_free() to call
slab_free_hook() directly instead of slab_free_freelist_hook().
If `init_on_free` is set, slab_free_hook() zeroes the object.
Afterward, if `slub_debug=F` and `CONFIG_SLAB_FREELIST_HARDENED` are
set, the do_slab_free() slowpath executes freelist consistency
checks and try to decode a zeroed freepointer which leads to a
"Freepointer corrupt" detection in check_object().
During bulk free, slab_free_freelist_hook() isn't affected as it always
sets it objects freepointer using set_freepointer() to maintain its
reconstructed freelist after `init_on_free`.
For single free, object's freepointer thus needs to be avoided when
stored outside the object if `init_on_free` is set. The freepointer left
as is, check_object() may later detect an invalid pointer value due to
objects overflow.
To reproduce, set `slub_debug=FU init_on_free=1 log_level=7` on the
command line of a kernel build with `CONFIG_SLAB_FREELIST_HARDENED=y`.
dmesg sample log:
[ 10.708715] =============================================================================
[ 10.710323] BUG kmalloc-rnd-05-32 (Tainted: G B T ): Freepointer corrupt
[ 10.712695] -----------------------------------------------------------------------------
[ 10.712695]
[ 10.712695] Slab 0xffffd8bdc400d580 objects=32 used=4 fp=0xffff9d9a80356f80 flags=0x200000000000a00(workingset|slab|node=0|zone=2)
[ 10.716698] Object 0xffff9d9a80356600 @offset=1536 fp=0x7ee4f480ce0ecd7c
[ 10.716698]
[ 10.716698] Bytes b4 ffff9d9a803565f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
[ 10.720703] Object ffff9d9a80356600: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
[ 10.720703] Object ffff9d9a80356610: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
[ 10.724696] Padding ffff9d9a8035666c: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
[ 10.724696] Padding ffff9d9a8035667c: 00 00 00 00 ....
[ 10.724696] FIX kmalloc-rnd-05-32: Object at 0xffff9d9a80356600 not freed
Co-developed-by: Chengming Zhou <chengming.zhou@linux.dev>
Signed-off-by: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
---
Changes since v2:
https://lore.kernel.org/all/ZjCxZfD1d36zfq-R@archlinux/
* Reword commit message in order to clarify the patch approach
as suggested by Vlastimil Babka
Changes since v1:
https://lore.kernel.org/all/Zij_fGjRS_rK-65r@archlinux/
* Jump above out of object freepointer if init_on_free is set
instead of initializing it with set_freepointer() as suggested
by Vlastimil Babka.
* Adapt maybe_wipe_obj_freeptr() to avoid wiping out of object
on alloc freepointer as suggested by Chengming Zhou.
* Reword commit message.
---
mm/slub.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/mm/slub.c b/mm/slub.c
index 3aa12b9b323d..173c340ec1d3 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -2102,15 +2102,20 @@ bool slab_free_hook(struct kmem_cache *s, void *x, bool init)
*
* The initialization memset's clear the object and the metadata,
* but don't touch the SLAB redzone.
+ *
+ * The object's freepointer is also avoided if stored outside the
+ * object.
*/
if (unlikely(init)) {
int rsize;
+ unsigned int inuse;
+ inuse = get_info_end(s);
if (!kasan_has_integrated_init())
memset(kasan_reset_tag(x), 0, s->object_size);
rsize = (s->flags & SLAB_RED_ZONE) ? s->red_left_pad : 0;
- memset((char *)kasan_reset_tag(x) + s->inuse, 0,
- s->size - s->inuse - rsize);
+ memset((char *)kasan_reset_tag(x) + inuse, 0,
+ s->size - inuse - rsize);
}
/* KASAN might put x into memory quarantine, delaying its reuse. */
return !kasan_slab_free(s, x, init);
@@ -3789,7 +3794,7 @@ static void *__slab_alloc_node(struct kmem_cache *s,
static __always_inline void maybe_wipe_obj_freeptr(struct kmem_cache *s,
void *obj)
{
- if (unlikely(slab_want_init_on_free(s)) && obj)
+ if (unlikely(slab_want_init_on_free(s)) && obj && !freeptr_outside_object(s))
memset((void *)((char *)kasan_reset_tag(obj) + s->offset),
0, sizeof(void *));
}
--
2.44.0
next reply other threads:[~2024-04-30 11:35 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-30 11:34 Nicolas Bouchinet [this message]
2024-04-30 11:58 ` [PATCH v3] slub: Fixes freepointer encoding for single free Vlastimil Babka
2024-04-30 12:24 ` Chengming Zhou
2024-08-19 2:43 ` Peng Fan
2024-08-26 19:37 ` Vlastimil Babka
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=ZjDXY74yS6UyQPxv@archlinux \
--to=nicolas.bouchinet@clip-os.org \
--cc=42.hyeyoo@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=chengming.zhou@linux.dev \
--cc=cl@linux.com \
--cc=iamjoonsoo.kim@lge.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=penberg@kernel.org \
--cc=rientjes@google.com \
--cc=roman.gushchin@linux.dev \
--cc=vbabka@suse.cz \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.