Re: [PATCH 2/2] mm: krealloc: clarify valid usage of __GFP_ZERO

[Danilo Krummrich <dakr@kernel.org>]

On Tue, Jul 30, 2024 at 01:35:40PM -0700, Andrew Morton wrote:
> On Tue, 30 Jul 2024 21:42:06 +0200 Danilo Krummrich <dakr@kernel.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.

Sounds reasonable, I'll add an explanation here and in the fixup series for
vrealloc() / kvrealloc().

> 
> 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?

For the case that is fixed in patch 1 of this series, no. KASAN can't detect
this.

As you say, the memory is just uninitialized (not poisoned), where it should
have been zeroed instead.