linux-kernel - Re: [PATCH 2/2] mm: krealloc: clarify valid usage of __GFP

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20240730133540.66e215082a513509c0d8649c@linux-foundation.org>
Date: Tue, 30 Jul 2024 13:35:40 -0700
From: Andrew Morton <akpm@...ux-foundation.org>
To: Danilo Krummrich <dakr@...nel.org>
Cc: cl@...ux.com, penberg@...nel.org, rientjes@...gle.com,
 iamjoonsoo.kim@....com, vbabka@...e.cz, roman.gushchin@...ux.dev,
 42.hyeyoo@...il.com, linux-kernel@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [PATCH 2/2] mm: krealloc: clarify valid usage of __GFP_ZERO

On Tue, 30 Jul 2024 21:42:06 +0200 Danilo Krummrich <dakr@...nel.org> wrote:

> Properly document that 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.
> 
> ...
>
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -733,6 +733,14 @@ static inline __alloc_size(1, 2) void *kmalloc_array_noprof(size_t n, size_t siz
>   * @new_n: new number of elements to alloc
>   * @new_size: new size of a single member of the array
>   * @flags: the type of memory to allocate (see kmalloc)
> + *
> + * 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.
>   */
>  static inline __realloc_size(2, 3) void * __must_check krealloc_array_noprof(void *p,
>  								       size_t new_n,
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index cff602cedf8e..faa13f42b111 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -1301,11 +1301,17 @@ __do_krealloc(const void *p, size_t new_size, gfp_t flags)
>   * @new_size: how many bytes of memory are required.
>   * @flags: the type of memory to allocate.
>   *
> - * The contents of the object pointed to are preserved up to the
> - * lesser of the new and old sizes (__GFP_ZERO flag is effectively ignored).
>   * 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.
> + *
> + * 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)

In both cases, we're saying "callers should do X".  I think it would be
better to say "this implementation does A, hence callers should do X". 
Tell people what's going on.

eg, "if krealloc is expanding an existing allocation, the newly-added
memory will be uninitialized unless the caller used __GFP_ZERO".  Or
something like that.

I assume that if the caller actually touches the uninitialized memory,
KASAN will warn?