| Message ID | 20240730194214.31483-2-dakr@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] mm: krealloc: consider spare memory for __GFP_ZERO | expand |
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. 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?
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.
diff --git a/include/linux/slab.h b/include/linux/slab.h index c9cb42203183..26f14c04000a 100644 --- 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)
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. Signed-off-by: Danilo Krummrich <dakr@kernel.org> --- include/linux/slab.h | 8 ++++++++ mm/slab_common.c | 10 ++++++++-- 2 files changed, 16 insertions(+), 2 deletions(-)