linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/2] Refine kmalloc caches randomization in kvmalloc
@ 2025-02-12  8:15 GONG Ruiqi
  2025-02-12  8:15 ` [PATCH v3 1/2] slab: Adjust placement of __kvmalloc_node_noprof GONG Ruiqi
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: GONG Ruiqi @ 2025-02-12  8:15 UTC (permalink / raw)
  To: Christoph Lameter, Pekka Enberg, David Rientjes, Joonsoo Kim,
	Andrew Morton, Vlastimil Babka, Kees Cook
  Cc: Tamas Koczka, Roman Gushchin, Hyeonggon Yoo, Xiu Jianfeng,
	linux-mm, linux-hardening, linux-kernel, gongruiqi1

Hi,

v3:
  - move all the way from kmalloc_gfp_adjust to kvrealloc_noprof into
    mm/slub.c
  - some rewording for commit logs
v2: https://lore.kernel.org/all/20250208014723.1514049-1-gongruiqi1@huawei.com/
  - change the implementation as Vlastimil suggested
v1: https://lore.kernel.org/all/20250122074817.991060-1-gongruiqi1@huawei.com/

Tamás reported [1] that kmalloc cache randomization doesn't actually
work for those kmalloc invoked via kvmalloc. For more details, see the
commit log of patch 2.

The current solution requires a direct call from __kvmalloc_node_noprof
to __do_kmalloc_node, a static function in a different .c file. As
suggested by Vlastimil [2], it's achieved by simply moving
__kvmalloc_node_noprof from mm/util.c to mm/slub.c, together with some
other functions of the same family.

Link: https://github.com/google/security-research/blob/908d59b573960dc0b90adda6f16f7017aca08609/pocs/linux/kernelctf/CVE-2024-27397_mitigation/docs/exploit.md?plain=1#L259 [1]
Link: https://lore.kernel.org/all/62044279-0c56-4185-97f7-7afac65ff449@suse.cz/ [2]

GONG Ruiqi (2):
  slab: Adjust placement of __kvmalloc_node_noprof
  slab: Achieve better kmalloc caches randomization in kvmalloc

 mm/slub.c | 162 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 mm/util.c | 162 ------------------------------------------------------
 2 files changed, 162 insertions(+), 162 deletions(-)

-- 
2.25.1



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v3 1/2] slab: Adjust placement of __kvmalloc_node_noprof
  2025-02-12  8:15 [PATCH v3 0/2] Refine kmalloc caches randomization in kvmalloc GONG Ruiqi
@ 2025-02-12  8:15 ` GONG Ruiqi
  2025-02-12  8:15 ` [PATCH v3 2/2] slab: Achieve better kmalloc caches randomization in kvmalloc GONG Ruiqi
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: GONG Ruiqi @ 2025-02-12  8:15 UTC (permalink / raw)
  To: Christoph Lameter, Pekka Enberg, David Rientjes, Joonsoo Kim,
	Andrew Morton, Vlastimil Babka, Kees Cook
  Cc: Tamas Koczka, Roman Gushchin, Hyeonggon Yoo, Xiu Jianfeng,
	linux-mm, linux-hardening, linux-kernel, gongruiqi1

Move __kvmalloc_node_noprof (as well as kvfree*, kvrealloc_noprof and
kmalloc_gfp_adjust for consistency) into mm/slub.c so that it can
directly invoke __do_kmalloc_node, which is needed for the next patch.

No functional changes intended.

Signed-off-by: GONG Ruiqi <gongruiqi1@huawei.com>
---
 mm/slub.c | 162 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 mm/util.c | 162 ------------------------------------------------------
 2 files changed, 162 insertions(+), 162 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index 1f50129dcfb3..abc982d68feb 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -4878,6 +4878,168 @@ void *krealloc_noprof(const void *p, size_t new_size, gfp_t flags)
 }
 EXPORT_SYMBOL(krealloc_noprof);
 
+static gfp_t kmalloc_gfp_adjust(gfp_t flags, size_t size)
+{
+	/*
+	 * We want to attempt a large physically contiguous block first because
+	 * it is less likely to fragment multiple larger blocks and therefore
+	 * contribute to a long term fragmentation less than vmalloc fallback.
+	 * However make sure that larger requests are not too disruptive - no
+	 * OOM killer and no allocation failure warnings as we have a fallback.
+	 */
+	if (size > PAGE_SIZE) {
+		flags |= __GFP_NOWARN;
+
+		if (!(flags & __GFP_RETRY_MAYFAIL))
+			flags |= __GFP_NORETRY;
+
+		/* nofail semantic is implemented by the vmalloc fallback */
+		flags &= ~__GFP_NOFAIL;
+	}
+
+	return flags;
+}
+
+/**
+ * __kvmalloc_node - attempt to allocate physically contiguous memory, but upon
+ * failure, fall back to non-contiguous (vmalloc) allocation.
+ * @size: size of the request.
+ * @b: which set of kmalloc buckets to allocate from.
+ * @flags: gfp mask for the allocation - must be compatible (superset) with GFP_KERNEL.
+ * @node: numa node to allocate from
+ *
+ * Uses kmalloc to get the memory but if the allocation fails then falls back
+ * to the vmalloc allocator. Use kvfree for freeing the memory.
+ *
+ * GFP_NOWAIT and GFP_ATOMIC are not supported, neither is the __GFP_NORETRY modifier.
+ * __GFP_RETRY_MAYFAIL is supported, and it should be used only if kmalloc is
+ * preferable to the vmalloc fallback, due to visible performance drawbacks.
+ *
+ * Return: pointer to the allocated memory of %NULL in case of failure
+ */
+void *__kvmalloc_node_noprof(DECL_BUCKET_PARAMS(size, b), gfp_t flags, int node)
+{
+	void *ret;
+
+	/*
+	 * It doesn't really make sense to fallback to vmalloc for sub page
+	 * requests
+	 */
+	ret = __kmalloc_node_noprof(PASS_BUCKET_PARAMS(size, b),
+				    kmalloc_gfp_adjust(flags, size),
+				    node);
+	if (ret || size <= PAGE_SIZE)
+		return ret;
+
+	/* non-sleeping allocations are not supported by vmalloc */
+	if (!gfpflags_allow_blocking(flags))
+		return NULL;
+
+	/* Don't even allow crazy sizes */
+	if (unlikely(size > INT_MAX)) {
+		WARN_ON_ONCE(!(flags & __GFP_NOWARN));
+		return NULL;
+	}
+
+	/*
+	 * kvmalloc() can always use VM_ALLOW_HUGE_VMAP,
+	 * since the callers already cannot assume anything
+	 * about the resulting pointer, and cannot play
+	 * protection games.
+	 */
+	return __vmalloc_node_range_noprof(size, 1, VMALLOC_START, VMALLOC_END,
+			flags, PAGE_KERNEL, VM_ALLOW_HUGE_VMAP,
+			node, __builtin_return_address(0));
+}
+EXPORT_SYMBOL(__kvmalloc_node_noprof);
+
+/**
+ * kvfree() - Free memory.
+ * @addr: Pointer to allocated memory.
+ *
+ * kvfree frees memory allocated by any of vmalloc(), kmalloc() or kvmalloc().
+ * It is slightly more efficient to use kfree() or vfree() if you are certain
+ * that you know which one to use.
+ *
+ * Context: Either preemptible task context or not-NMI interrupt.
+ */
+void kvfree(const void *addr)
+{
+	if (is_vmalloc_addr(addr))
+		vfree(addr);
+	else
+		kfree(addr);
+}
+EXPORT_SYMBOL(kvfree);
+
+/**
+ * kvfree_sensitive - Free a data object containing sensitive information.
+ * @addr: address of the data object to be freed.
+ * @len: length of the data object.
+ *
+ * Use the special memzero_explicit() function to clear the content of a
+ * kvmalloc'ed object containing sensitive data to make sure that the
+ * compiler won't optimize out the data clearing.
+ */
+void kvfree_sensitive(const void *addr, size_t len)
+{
+	if (likely(!ZERO_OR_NULL_PTR(addr))) {
+		memzero_explicit((void *)addr, len);
+		kvfree(addr);
+	}
+}
+EXPORT_SYMBOL(kvfree_sensitive);
+
+/**
+ * kvrealloc - reallocate memory; contents remain unchanged
+ * @p: object to reallocate memory for
+ * @size: the size to reallocate
+ * @flags: the flags for the page level allocator
+ *
+ * If @p is %NULL, kvrealloc() behaves exactly like kvmalloc(). If @size is 0
+ * and @p is not a %NULL pointer, the object pointed to is freed.
+ *
+ * If __GFP_ZERO logic is requested, callers must ensure that, starting with the
+ * initial memory allocation, every subsequent call to this API for the same
+ * memory allocation is flagged with __GFP_ZERO. Otherwise, it is possible that
+ * __GFP_ZERO is not fully honored by this API.
+ *
+ * In any case, the contents of the object pointed to are preserved up to the
+ * lesser of the new and old sizes.
+ *
+ * This function must not be called concurrently with itself or kvfree() for the
+ * same memory allocation.
+ *
+ * Return: pointer to the allocated memory or %NULL in case of error
+ */
+void *kvrealloc_noprof(const void *p, size_t size, gfp_t flags)
+{
+	void *n;
+
+	if (is_vmalloc_addr(p))
+		return vrealloc_noprof(p, size, flags);
+
+	n = krealloc_noprof(p, size, kmalloc_gfp_adjust(flags, size));
+	if (!n) {
+		/* We failed to krealloc(), fall back to kvmalloc(). */
+		n = kvmalloc_noprof(size, flags);
+		if (!n)
+			return NULL;
+
+		if (p) {
+			/* We already know that `p` is not a vmalloc address. */
+			kasan_disable_current();
+			memcpy(n, kasan_reset_tag(p), ksize(p));
+			kasan_enable_current();
+
+			kfree(p);
+		}
+	}
+
+	return n;
+}
+EXPORT_SYMBOL(kvrealloc_noprof);
+
 struct detached_freelist {
 	struct slab *slab;
 	void *tail;
diff --git a/mm/util.c b/mm/util.c
index b6b9684a1438..c808668f0548 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -612,168 +612,6 @@ unsigned long vm_mmap(struct file *file, unsigned long addr,
 }
 EXPORT_SYMBOL(vm_mmap);
 
-static gfp_t kmalloc_gfp_adjust(gfp_t flags, size_t size)
-{
-	/*
-	 * We want to attempt a large physically contiguous block first because
-	 * it is less likely to fragment multiple larger blocks and therefore
-	 * contribute to a long term fragmentation less than vmalloc fallback.
-	 * However make sure that larger requests are not too disruptive - no
-	 * OOM killer and no allocation failure warnings as we have a fallback.
-	 */
-	if (size > PAGE_SIZE) {
-		flags |= __GFP_NOWARN;
-
-		if (!(flags & __GFP_RETRY_MAYFAIL))
-			flags |= __GFP_NORETRY;
-
-		/* nofail semantic is implemented by the vmalloc fallback */
-		flags &= ~__GFP_NOFAIL;
-	}
-
-	return flags;
-}
-
-/**
- * __kvmalloc_node - attempt to allocate physically contiguous memory, but upon
- * failure, fall back to non-contiguous (vmalloc) allocation.
- * @size: size of the request.
- * @b: which set of kmalloc buckets to allocate from.
- * @flags: gfp mask for the allocation - must be compatible (superset) with GFP_KERNEL.
- * @node: numa node to allocate from
- *
- * Uses kmalloc to get the memory but if the allocation fails then falls back
- * to the vmalloc allocator. Use kvfree for freeing the memory.
- *
- * GFP_NOWAIT and GFP_ATOMIC are not supported, neither is the __GFP_NORETRY modifier.
- * __GFP_RETRY_MAYFAIL is supported, and it should be used only if kmalloc is
- * preferable to the vmalloc fallback, due to visible performance drawbacks.
- *
- * Return: pointer to the allocated memory of %NULL in case of failure
- */
-void *__kvmalloc_node_noprof(DECL_BUCKET_PARAMS(size, b), gfp_t flags, int node)
-{
-	void *ret;
-
-	/*
-	 * It doesn't really make sense to fallback to vmalloc for sub page
-	 * requests
-	 */
-	ret = __kmalloc_node_noprof(PASS_BUCKET_PARAMS(size, b),
-				    kmalloc_gfp_adjust(flags, size),
-				    node);
-	if (ret || size <= PAGE_SIZE)
-		return ret;
-
-	/* non-sleeping allocations are not supported by vmalloc */
-	if (!gfpflags_allow_blocking(flags))
-		return NULL;
-
-	/* Don't even allow crazy sizes */
-	if (unlikely(size > INT_MAX)) {
-		WARN_ON_ONCE(!(flags & __GFP_NOWARN));
-		return NULL;
-	}
-
-	/*
-	 * kvmalloc() can always use VM_ALLOW_HUGE_VMAP,
-	 * since the callers already cannot assume anything
-	 * about the resulting pointer, and cannot play
-	 * protection games.
-	 */
-	return __vmalloc_node_range_noprof(size, 1, VMALLOC_START, VMALLOC_END,
-			flags, PAGE_KERNEL, VM_ALLOW_HUGE_VMAP,
-			node, __builtin_return_address(0));
-}
-EXPORT_SYMBOL(__kvmalloc_node_noprof);
-
-/**
- * kvfree() - Free memory.
- * @addr: Pointer to allocated memory.
- *
- * kvfree frees memory allocated by any of vmalloc(), kmalloc() or kvmalloc().
- * It is slightly more efficient to use kfree() or vfree() if you are certain
- * that you know which one to use.
- *
- * Context: Either preemptible task context or not-NMI interrupt.
- */
-void kvfree(const void *addr)
-{
-	if (is_vmalloc_addr(addr))
-		vfree(addr);
-	else
-		kfree(addr);
-}
-EXPORT_SYMBOL(kvfree);
-
-/**
- * kvfree_sensitive - Free a data object containing sensitive information.
- * @addr: address of the data object to be freed.
- * @len: length of the data object.
- *
- * Use the special memzero_explicit() function to clear the content of a
- * kvmalloc'ed object containing sensitive data to make sure that the
- * compiler won't optimize out the data clearing.
- */
-void kvfree_sensitive(const void *addr, size_t len)
-{
-	if (likely(!ZERO_OR_NULL_PTR(addr))) {
-		memzero_explicit((void *)addr, len);
-		kvfree(addr);
-	}
-}
-EXPORT_SYMBOL(kvfree_sensitive);
-
-/**
- * kvrealloc - reallocate memory; contents remain unchanged
- * @p: object to reallocate memory for
- * @size: the size to reallocate
- * @flags: the flags for the page level allocator
- *
- * If @p is %NULL, kvrealloc() behaves exactly like kvmalloc(). If @size is 0
- * and @p is not a %NULL pointer, the object pointed to is freed.
- *
- * If __GFP_ZERO logic is requested, callers must ensure that, starting with the
- * initial memory allocation, every subsequent call to this API for the same
- * memory allocation is flagged with __GFP_ZERO. Otherwise, it is possible that
- * __GFP_ZERO is not fully honored by this API.
- *
- * In any case, the contents of the object pointed to are preserved up to the
- * lesser of the new and old sizes.
- *
- * This function must not be called concurrently with itself or kvfree() for the
- * same memory allocation.
- *
- * Return: pointer to the allocated memory or %NULL in case of error
- */
-void *kvrealloc_noprof(const void *p, size_t size, gfp_t flags)
-{
-	void *n;
-
-	if (is_vmalloc_addr(p))
-		return vrealloc_noprof(p, size, flags);
-
-	n = krealloc_noprof(p, size, kmalloc_gfp_adjust(flags, size));
-	if (!n) {
-		/* We failed to krealloc(), fall back to kvmalloc(). */
-		n = kvmalloc_noprof(size, flags);
-		if (!n)
-			return NULL;
-
-		if (p) {
-			/* We already know that `p` is not a vmalloc address. */
-			kasan_disable_current();
-			memcpy(n, kasan_reset_tag(p), ksize(p));
-			kasan_enable_current();
-
-			kfree(p);
-		}
-	}
-
-	return n;
-}
-EXPORT_SYMBOL(kvrealloc_noprof);
-
 /**
  * __vmalloc_array - allocate memory for a virtually contiguous array.
  * @n: number of elements.
-- 
2.25.1



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v3 2/2] slab: Achieve better kmalloc caches randomization in kvmalloc
  2025-02-12  8:15 [PATCH v3 0/2] Refine kmalloc caches randomization in kvmalloc GONG Ruiqi
  2025-02-12  8:15 ` [PATCH v3 1/2] slab: Adjust placement of __kvmalloc_node_noprof GONG Ruiqi
@ 2025-02-12  8:15 ` GONG Ruiqi
  2025-02-12 14:20 ` [PATCH v3 0/2] Refine " Harry Yoo
  2025-02-12 15:12 ` Vlastimil Babka
  3 siblings, 0 replies; 6+ messages in thread
From: GONG Ruiqi @ 2025-02-12  8:15 UTC (permalink / raw)
  To: Christoph Lameter, Pekka Enberg, David Rientjes, Joonsoo Kim,
	Andrew Morton, Vlastimil Babka, Kees Cook
  Cc: Tamas Koczka, Roman Gushchin, Hyeonggon Yoo, Xiu Jianfeng,
	linux-mm, linux-hardening, linux-kernel, gongruiqi1

As revealed by this writeup[1], due to the fact that __kmalloc_node (now
renamed to __kmalloc_node_noprof) is an exported symbol and will never
get inlined, using it in kvmalloc_node (now is __kvmalloc_node_noprof)
would make the RET_IP inside always point to the same address:

    upper_caller
        kvmalloc
        kvmalloc_node
        kvmalloc_node_noprof
        __kvmalloc_node_noprof	<-- all macros all the way down here
            __kmalloc_node_noprof
                __do_kmalloc_node(.., _RET_IP_)
            ...			<-- _RET_IP_ points to

That literally means all kmalloc invoked via kvmalloc would use the same
seed for cache randomization (CONFIG_RANDOM_KMALLOC_CACHES), which makes
this hardening non-functional.

The root cause of this problem, IMHO, is that using RET_IP only cannot
identify the actual allocation site in case of kmalloc being called
inside non-inlined wrappers or helper functions. And I believe there
could be similar cases in other functions. Nevertheless, I haven't
thought of any good solution for this. So for now let's solve this
specific case first.

For __kvmalloc_node_noprof, replace __kmalloc_node_noprof and call
__do_kmalloc_node directly instead, so that RET_IP can take the return
address of kvmalloc and differentiate each kvmalloc invocation:

    upper_caller
        kvmalloc
        kvmalloc_node
        kvmalloc_node_noprof
        __kvmalloc_node_noprof	<-- all macros all the way down here
            __do_kmalloc_node(.., _RET_IP_)
        ...			<-- _RET_IP_ points to

Thanks to Tamás Koczka for the report and discussion!

Link: https://github.com/google/security-research/blob/908d59b573960dc0b90adda6f16f7017aca08609/pocs/linux/kernelctf/CVE-2024-27397_mitigation/docs/exploit.md?plain=1#L259 [1]
Reported-by: Tamás Koczka <poprdi@google.com>
Signed-off-by: GONG Ruiqi <gongruiqi1@huawei.com>
---
 mm/slub.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index abc982d68feb..1f7d1d260eeb 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -4925,9 +4925,9 @@ void *__kvmalloc_node_noprof(DECL_BUCKET_PARAMS(size, b), gfp_t flags, int node)
 	 * It doesn't really make sense to fallback to vmalloc for sub page
 	 * requests
 	 */
-	ret = __kmalloc_node_noprof(PASS_BUCKET_PARAMS(size, b),
-				    kmalloc_gfp_adjust(flags, size),
-				    node);
+	ret = __do_kmalloc_node(size, PASS_BUCKET_PARAM(b),
+				kmalloc_gfp_adjust(flags, size),
+				node, _RET_IP_);
 	if (ret || size <= PAGE_SIZE)
 		return ret;
 
-- 
2.25.1



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 0/2] Refine kmalloc caches randomization in kvmalloc
  2025-02-12  8:15 [PATCH v3 0/2] Refine kmalloc caches randomization in kvmalloc GONG Ruiqi
  2025-02-12  8:15 ` [PATCH v3 1/2] slab: Adjust placement of __kvmalloc_node_noprof GONG Ruiqi
  2025-02-12  8:15 ` [PATCH v3 2/2] slab: Achieve better kmalloc caches randomization in kvmalloc GONG Ruiqi
@ 2025-02-12 14:20 ` Harry Yoo
  2025-02-12 14:32   ` Vlastimil Babka
  2025-02-12 15:12 ` Vlastimil Babka
  3 siblings, 1 reply; 6+ messages in thread
From: Harry Yoo @ 2025-02-12 14:20 UTC (permalink / raw)
  To: GONG Ruiqi
  Cc: Christoph Lameter, Pekka Enberg, David Rientjes, Joonsoo Kim,
	Andrew Morton, Vlastimil Babka, Kees Cook, Tamas Koczka,
	Roman Gushchin, Xiu Jianfeng, linux-mm, linux-hardening,
	linux-kernel

On Wed, Feb 12, 2025 at 04:15:03PM +0800, GONG Ruiqi wrote:
> Hi,
> 
> v3:
>   - move all the way from kmalloc_gfp_adjust to kvrealloc_noprof into
>     mm/slub.c
>   - some rewording for commit logs
> v2: https://lore.kernel.org/all/20250208014723.1514049-1-gongruiqi1@huawei.com/
>   - change the implementation as Vlastimil suggested
> v1: https://lore.kernel.org/all/20250122074817.991060-1-gongruiqi1@huawei.com/
> 
> Tamás reported [1] that kmalloc cache randomization doesn't actually
> work for those kmalloc invoked via kvmalloc. For more details, see the
> commit log of patch 2.
> 
> The current solution requires a direct call from __kvmalloc_node_noprof
> to __do_kmalloc_node, a static function in a different .c file. As
> suggested by Vlastimil [2], it's achieved by simply moving
> __kvmalloc_node_noprof from mm/util.c to mm/slub.c, together with some
> other functions of the same family.

Hi, GONG!
Sorry for my late review.

This patch series looks good to me (with a nit),
Reviewed-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>

Also, I verified that the problem you described exists on slab/for-next,
and the patch series fixes the problem. Please feel free to add,
Tested-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>

nit: Does it make sense to call __kvmalloc_node_track_caller_noprof()
instead of __do_kmalloc_node() to avoid bloating the code size?

My simple build test says it saves 1592 bytes:
  $ ./scripts/bloat-o-meter slub.o.before slub.o.after
  add/remove: 0/1 grow/shrink: 0/1 up/down: 0/-1592 (-1592)
  Function                                     old     new   delta
  __kvmalloc_node_noprof.cold                   39       -     -39
  __kvmalloc_node_noprof                      1755     202   -1553
  Total: Before=79723, After=78131, chg -2.00%

> Link: https://github.com/google/security-research/blob/908d59b573960dc0b90adda6f16f7017aca08609/pocs/linux/kernelctf/CVE-2024-27397_mitigation/docs/exploit.md?plain=1#L259 [1]
> Link: https://lore.kernel.org/all/62044279-0c56-4185-97f7-7afac65ff449@suse.cz/ [2]

-- 
Harry


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 0/2] Refine kmalloc caches randomization in kvmalloc
  2025-02-12 14:20 ` [PATCH v3 0/2] Refine " Harry Yoo
@ 2025-02-12 14:32   ` Vlastimil Babka
  0 siblings, 0 replies; 6+ messages in thread
From: Vlastimil Babka @ 2025-02-12 14:32 UTC (permalink / raw)
  To: Harry Yoo, GONG Ruiqi
  Cc: Christoph Lameter, Pekka Enberg, David Rientjes, Joonsoo Kim,
	Andrew Morton, Kees Cook, Tamas Koczka, Roman Gushchin,
	Xiu Jianfeng, linux-mm, linux-hardening, linux-kernel

On 2/12/25 15:20, Harry Yoo wrote:
> On Wed, Feb 12, 2025 at 04:15:03PM +0800, GONG Ruiqi wrote:
>> Hi,
>> 
>> v3:
>>   - move all the way from kmalloc_gfp_adjust to kvrealloc_noprof into
>>     mm/slub.c
>>   - some rewording for commit logs
>> v2: https://lore.kernel.org/all/20250208014723.1514049-1-gongruiqi1@huawei.com/
>>   - change the implementation as Vlastimil suggested
>> v1: https://lore.kernel.org/all/20250122074817.991060-1-gongruiqi1@huawei.com/
>> 
>> Tamás reported [1] that kmalloc cache randomization doesn't actually
>> work for those kmalloc invoked via kvmalloc. For more details, see the
>> commit log of patch 2.
>> 
>> The current solution requires a direct call from __kvmalloc_node_noprof
>> to __do_kmalloc_node, a static function in a different .c file. As
>> suggested by Vlastimil [2], it's achieved by simply moving
>> __kvmalloc_node_noprof from mm/util.c to mm/slub.c, together with some
>> other functions of the same family.
> 
> Hi, GONG!
> Sorry for my late review.
> 
> This patch series looks good to me (with a nit),
> Reviewed-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
> 
> Also, I verified that the problem you described exists on slab/for-next,
> and the patch series fixes the problem. Please feel free to add,
> Tested-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>

Thanks!

> nit: Does it make sense to call __kvmalloc_node_track_caller_noprof()
> instead of __do_kmalloc_node() to avoid bloating the code size?

Hm I think it would be a bit arbitrary to make kvmalloc special like this
here. But we should probably change __do_kmalloc_node() to
__fastpath_inline. Or even check if not making it inline at all results in
other callers (not kvmalloc probably due to its complexity) doing a tail
call there, which should be fast enough.

> My simple build test says it saves 1592 bytes:
>   $ ./scripts/bloat-o-meter slub.o.before slub.o.after
>   add/remove: 0/1 grow/shrink: 0/1 up/down: 0/-1592 (-1592)
>   Function                                     old     new   delta
>   __kvmalloc_node_noprof.cold                   39       -     -39
>   __kvmalloc_node_noprof                      1755     202   -1553
>   Total: Before=79723, After=78131, chg -2.00%
> 
>> Link: https://github.com/google/security-research/blob/908d59b573960dc0b90adda6f16f7017aca08609/pocs/linux/kernelctf/CVE-2024-27397_mitigation/docs/exploit.md?plain=1#L259 [1]
>> Link: https://lore.kernel.org/all/62044279-0c56-4185-97f7-7afac65ff449@suse.cz/ [2]
> 



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 0/2] Refine kmalloc caches randomization in kvmalloc
  2025-02-12  8:15 [PATCH v3 0/2] Refine kmalloc caches randomization in kvmalloc GONG Ruiqi
                   ` (2 preceding siblings ...)
  2025-02-12 14:20 ` [PATCH v3 0/2] Refine " Harry Yoo
@ 2025-02-12 15:12 ` Vlastimil Babka
  3 siblings, 0 replies; 6+ messages in thread
From: Vlastimil Babka @ 2025-02-12 15:12 UTC (permalink / raw)
  To: GONG Ruiqi, Christoph Lameter, Pekka Enberg, David Rientjes,
	Joonsoo Kim, Andrew Morton, Kees Cook
  Cc: Tamas Koczka, Roman Gushchin, Hyeonggon Yoo, Xiu Jianfeng,
	linux-mm, linux-hardening, linux-kernel

On 2/12/25 09:15, GONG Ruiqi wrote:
> Hi,
> 
> v3:
>   - move all the way from kmalloc_gfp_adjust to kvrealloc_noprof into
>     mm/slub.c
>   - some rewording for commit logs
> v2: https://lore.kernel.org/all/20250208014723.1514049-1-gongruiqi1@huawei.com/
>   - change the implementation as Vlastimil suggested
> v1: https://lore.kernel.org/all/20250122074817.991060-1-gongruiqi1@huawei.com/
> 
> Tamás reported [1] that kmalloc cache randomization doesn't actually
> work for those kmalloc invoked via kvmalloc. For more details, see the
> commit log of patch 2.
> 
> The current solution requires a direct call from __kvmalloc_node_noprof
> to __do_kmalloc_node, a static function in a different .c file. As
> suggested by Vlastimil [2], it's achieved by simply moving
> __kvmalloc_node_noprof from mm/util.c to mm/slub.c, together with some
> other functions of the same family.
> 
> Link: https://github.com/google/security-research/blob/908d59b573960dc0b90adda6f16f7017aca08609/pocs/linux/kernelctf/CVE-2024-27397_mitigation/docs/exploit.md?plain=1#L259 [1]
> Link: https://lore.kernel.org/all/62044279-0c56-4185-97f7-7afac65ff449@suse.cz/ [2]
> 
> GONG Ruiqi (2):
>   slab: Adjust placement of __kvmalloc_node_noprof
>   slab: Achieve better kmalloc caches randomization in kvmalloc

Applied to slab/for-next, thanks!

> 
>  mm/slub.c | 162 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  mm/util.c | 162 ------------------------------------------------------
>  2 files changed, 162 insertions(+), 162 deletions(-)
> 



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-02-12 15:12 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-02-12  8:15 [PATCH v3 0/2] Refine kmalloc caches randomization in kvmalloc GONG Ruiqi
2025-02-12  8:15 ` [PATCH v3 1/2] slab: Adjust placement of __kvmalloc_node_noprof GONG Ruiqi
2025-02-12  8:15 ` [PATCH v3 2/2] slab: Achieve better kmalloc caches randomization in kvmalloc GONG Ruiqi
2025-02-12 14:20 ` [PATCH v3 0/2] Refine " Harry Yoo
2025-02-12 14:32   ` Vlastimil Babka
2025-02-12 15:12 ` Vlastimil Babka

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox