| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <ZuAaDbSMtpLVJPrY@pollux>
Date: Tue, 10 Sep 2024 12:06:05 +0200
From: Danilo Krummrich <dakr@...nel.org>
To: Feng Tang <feng.tang@...el.com>
Cc: Vlastimil Babka <vbabka@...e.cz>,
Andrew Morton <akpm@...ux-foundation.org>,
Christoph Lameter <cl@...ux.com>, Pekka Enberg <penberg@...nel.org>,
David Rientjes <rientjes@...gle.com>,
Joonsoo Kim <iamjoonsoo.kim@....com>,
Roman Gushchin <roman.gushchin@...ux.dev>,
Hyeonggon Yoo <42.hyeyoo@...il.com>,
Andrey Konovalov <andreyknvl@...il.com>,
Marco Elver <elver@...gle.com>,
Shuah Khan <skhan@...uxfoundation.org>,
David Gow <davidgow@...gle.com>, linux-mm@...ck.org,
kasan-dev@...glegroups.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/5] mm/slub: Improve redzone check and zeroing for
krealloc()
On Mon, Sep 09, 2024 at 09:29:56AM +0800, Feng Tang wrote:
> For current krealloc(), one problem is its caller doesn't know what's
> the actual request size, say the object is 64 bytes kmalloc one, but
> the original caller may only requested 48 bytes. And when krealloc()
> shrinks or grows in the same object, or allocate a new bigger object,
> it lacks this 'original size' information to do accurate data preserving
> or zeroing (when __GFP_ZERO is set).
>
> And when some slub debug option is enabled, kmalloc caches do have this
> 'orig_size' feature. So utilize it to do more accurate data handling,
> as well as enforce the kmalloc-redzone sanity check.
>
> The krealloc() related code is moved from slab_common.c to slub.c for
> more efficient function calling.
I think it would be good to do this in a separate commit, for a better diff and
history.
>
> Suggested-by: Vlastimil Babka <vbabka@...e.cz>
> Signed-off-by: Feng Tang <feng.tang@...el.com>
> ---
> mm/slab_common.c | 84 -------------------------------------
> mm/slub.c | 106 +++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 106 insertions(+), 84 deletions(-)
>
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index ad438ba62485..e59942fb7970 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -1297,90 +1297,6 @@ module_init(slab_proc_init);
>
> #endif /* CONFIG_SLUB_DEBUG */
>
> -static __always_inline __realloc_size(2) void *
> -__do_krealloc(const void *p, size_t new_size, gfp_t flags)
> -{
> - void *ret;
> - size_t ks;
> -
> - /* Check for double-free before calling ksize. */
> - if (likely(!ZERO_OR_NULL_PTR(p))) {
> - if (!kasan_check_byte(p))
> - return NULL;
> - ks = ksize(p);
> - } else
> - ks = 0;
> -
> - /* If the object still fits, repoison it precisely. */
> - if (ks >= new_size) {
> - /* Zero out spare memory. */
> - if (want_init_on_alloc(flags)) {
> - kasan_disable_current();
> - memset((void *)p + new_size, 0, ks - new_size);
> - kasan_enable_current();
> - }
> -
> - p = kasan_krealloc((void *)p, new_size, flags);
> - return (void *)p;
> - }
> -
> - ret = kmalloc_node_track_caller_noprof(new_size, flags, NUMA_NO_NODE, _RET_IP_);
> - if (ret && p) {
> - /* Disable KASAN checks as the object's redzone is accessed. */
> - kasan_disable_current();
> - memcpy(ret, kasan_reset_tag(p), ks);
> - kasan_enable_current();
> - }
> -
> - return ret;
> -}
> -
> -/**
> - * krealloc - reallocate memory. The contents will remain unchanged.
> - * @p: object to reallocate memory for.
> - * @new_size: how many bytes of memory are required.
> - * @flags: the type of memory to allocate.
> - *
> - * If @p is %NULL, krealloc() behaves exactly like kmalloc(). If @new_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.
> - *
> - * This is the case, since krealloc() only knows about the bucket size of an
> - * allocation (but not the exact size it was allocated with) and hence
> - * implements the following semantics for shrinking and growing buffers with
> - * __GFP_ZERO.
> - *
> - * new bucket
> - * 0 size size
> - * |--------|----------------|
> - * | keep | zero |
> - *
> - * In any case, the contents of the object pointed to are preserved up to the
> - * lesser of the new and old sizes.
> - *
> - * Return: pointer to the allocated memory or %NULL in case of error
> - */
> -void *krealloc_noprof(const void *p, size_t new_size, gfp_t flags)
> -{
> - void *ret;
> -
> - if (unlikely(!new_size)) {
> - kfree(p);
> - return ZERO_SIZE_PTR;
> - }
> -
> - ret = __do_krealloc(p, new_size, flags);
> - if (ret && kasan_reset_tag(p) != kasan_reset_tag(ret))
> - kfree(p);
> -
> - return ret;
> -}
> -EXPORT_SYMBOL(krealloc_noprof);
> -
> /**
> * kfree_sensitive - Clear sensitive information in memory before freeing
> * @p: object to free memory of
> diff --git a/mm/slub.c b/mm/slub.c
> index 4cb3822dba08..d4c938dfb89e 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -4709,6 +4709,112 @@ void kfree(const void *object)
> }
> EXPORT_SYMBOL(kfree);
>
> +static __always_inline __realloc_size(2) void *
> +__do_krealloc(const void *p, size_t new_size, gfp_t flags)
> +{
> + void *ret;
> + size_t ks;
> + int orig_size = 0;
> + struct kmem_cache *s;
> +
> + /* Check for double-free before calling ksize. */
> + if (likely(!ZERO_OR_NULL_PTR(p))) {
> + if (!kasan_check_byte(p))
> + return NULL;
> +
> + s = virt_to_cache(p);
> + orig_size = get_orig_size(s, (void *)p);
> + ks = s->object_size;
> + } else
> + ks = 0;
> +
> + /* If the object doesn't fit, allocate a bigger one */
> + if (new_size > ks)
> + goto alloc_new;
> +
> + /* Zero out spare memory. */
> + if (want_init_on_alloc(flags)) {
> + kasan_disable_current();
> + if (orig_size < new_size)
> + memset((void *)p + orig_size, 0, new_size - orig_size);
> + else
> + memset((void *)p + new_size, 0, ks - new_size);
> + kasan_enable_current();
> + }
> +
> + if (slub_debug_orig_size(s) && !is_kfence_address(p)) {
> + set_orig_size(s, (void *)p, new_size);
> + if (s->flags & SLAB_RED_ZONE && new_size < ks)
> + memset_no_sanitize_memory((void *)p + new_size,
> + SLUB_RED_ACTIVE, ks - new_size);
> + }
> +
> + p = kasan_krealloc((void *)p, new_size, flags);
> + return (void *)p;
> +
> +alloc_new:
> + ret = kmalloc_node_track_caller_noprof(new_size, flags, NUMA_NO_NODE, _RET_IP_);
> + if (ret && p) {
> + /* Disable KASAN checks as the object's redzone is accessed. */
> + kasan_disable_current();
> + if (orig_size)
> + memcpy(ret, kasan_reset_tag(p), orig_size);
> + kasan_enable_current();
> + }
> +
> + return ret;
> +}
> +
> +/**
> + * krealloc - reallocate memory. The contents will remain unchanged.
> + * @p: object to reallocate memory for.
> + * @new_size: how many bytes of memory are required.
> + * @flags: the type of memory to allocate.
> + *
> + * If @p is %NULL, krealloc() behaves exactly like kmalloc(). If @new_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.
> + *
> + * When slub_debug_orig_size() is off, since krealloc() only knows about the
I think you want to remove ' since ' here.
> + * bucket size of an allocation (but not the exact size it was allocated with)
> + * and hence implements the following semantics for shrinking and growing
> + * buffers with __GFP_ZERO.
> + *
> + * new bucket
> + * 0 size size
> + * |--------|----------------|
> + * | keep | zero |
> + *
> + * Otherwize, the original allocation size 'orig_size' could be used to
Typo in 'otherwise'.
> + * precisely clear the requested size, and the new size will also be stored as
> + * the new 'orig_size'.
> + *
> + * In any case, the contents of the object pointed to are preserved up to the
> + * lesser of the new and old sizes.
> + *
> + * Return: pointer to the allocated memory or %NULL in case of error
> + */
> +void *krealloc_noprof(const void *p, size_t new_size, gfp_t flags)
> +{
> + void *ret;
> +
> + if (unlikely(!new_size)) {
> + kfree(p);
> + return ZERO_SIZE_PTR;
> + }
> +
> + ret = __do_krealloc(p, new_size, flags);
> + if (ret && kasan_reset_tag(p) != kasan_reset_tag(ret))
> + kfree(p);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(krealloc_noprof);
> +
> struct detached_freelist {
> struct slab *slab;
> void *tail;
> --
> 2.34.1
>
Powered by blists - more mailing lists