| Message ID | 20220922031013.2150682-12-keescook@chromium.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | slab: Introduce kmalloc_size_roundup() | expand |
On Thu, Sep 22, 2022 at 5:10 AM Kees Cook <keescook@chromium.org> wrote: > > -#ifdef __alloc_size__ > -# define __alloc_size(x, ...) __alloc_size__(x, ## __VA_ARGS__) __malloc > -#else > -# define __alloc_size(x, ...) __malloc > -#endif > +#define __alloc_size(x, ...) __alloc_size__(x, ## __VA_ARGS__) __malloc > +#define __realloc_size(x, ...) __alloc_size__(x, ## __VA_ARGS__) These look unconditional now, so we could move it to `compiler_attributes.h` in a later patch (or an independent series). Cheers, Miguel
On Thu, Sep 22, 2022 at 11:23:46AM +0200, Miguel Ojeda wrote: > On Thu, Sep 22, 2022 at 5:10 AM Kees Cook <keescook@chromium.org> wrote: > > > > -#ifdef __alloc_size__ > > -# define __alloc_size(x, ...) __alloc_size__(x, ## __VA_ARGS__) __malloc > > -#else > > -# define __alloc_size(x, ...) __malloc > > -#endif > > +#define __alloc_size(x, ...) __alloc_size__(x, ## __VA_ARGS__) __malloc > > +#define __realloc_size(x, ...) __alloc_size__(x, ## __VA_ARGS__) > > These look unconditional now, so we could move it to > `compiler_attributes.h` in a later patch (or an independent series). I wasn't sure if this "composite macro" was sane there, especially since it would be using __malloc before it was defined, etc. Would you prefer I move it?
On Thu, Sep 22, 2022 at 5:56 PM Kees Cook <keescook@chromium.org> wrote: > > I wasn't sure if this "composite macro" was sane there, especially since > it would be using __malloc before it was defined, etc. Would you prefer > I move it? Hmm... On one hand, they end up being attributes, so it could make sense to have them there (after all, the big advantage of that header is that there is no `#ifdef` nest like in others, and that it is only for attributes). On the other hand, you are right that the file so far is intended to be as simple as possible (`__always_inline` having an extra `inline` and `fallthrough` would be closest outliers), so if we do it, I would prefer to do so in an independent series that carries its own rationale. So I would leave the patch as it is here. Cheers, Miguel
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h index 4f2a819fd60a..f141a6f6b9f6 100644 --- a/include/linux/compiler_types.h +++ b/include/linux/compiler_types.h @@ -271,15 +271,12 @@ struct ftrace_likely_data { /* * Any place that could be marked with the "alloc_size" attribute is also - * a place to be marked with the "malloc" attribute. Do this as part of the - * __alloc_size macro to avoid redundant attributes and to avoid missing a - * __malloc marking. + * a place to be marked with the "malloc" attribute, except those that may + * be performing a _reallocation_, as that may alias the existing pointer. + * For these, use __realloc_size(). */ -#ifdef __alloc_size__ -# define __alloc_size(x, ...) __alloc_size__(x, ## __VA_ARGS__) __malloc -#else -# define __alloc_size(x, ...) __malloc -#endif +#define __alloc_size(x, ...) __alloc_size__(x, ## __VA_ARGS__) __malloc +#define __realloc_size(x, ...) __alloc_size__(x, ## __VA_ARGS__) #ifndef asm_volatile_goto #define asm_volatile_goto(x...) asm goto(x) diff --git a/include/linux/slab.h b/include/linux/slab.h index 4fc41e4ed4a2..ac3832b50dbb 100644 --- a/include/linux/slab.h +++ b/include/linux/slab.h @@ -184,7 +184,7 @@ int kmem_cache_shrink(struct kmem_cache *s); /* * Common kmalloc functions provided by all allocators */ -void * __must_check krealloc(const void *objp, size_t new_size, gfp_t flags) __alloc_size(2); +void * __must_check krealloc(const void *objp, size_t new_size, gfp_t flags) __realloc_size(2); void kfree(const void *objp); void kfree_sensitive(const void *objp); size_t __ksize(const void *objp); @@ -661,10 +661,10 @@ static inline __alloc_size(1, 2) void *kmalloc_array(size_t n, size_t size, gfp_ * @new_size: new size of a single member of the array * @flags: the type of memory to allocate (see kmalloc) */ -static inline __alloc_size(2, 3) void * __must_check krealloc_array(void *p, - size_t new_n, - size_t new_size, - gfp_t flags) +static inline __realloc_size(2, 3) void * __must_check krealloc_array(void *p, + size_t new_n, + size_t new_size, + gfp_t flags) { size_t bytes; @@ -788,7 +788,7 @@ static inline __alloc_size(1, 2) void *kvcalloc(size_t n, size_t size, gfp_t fla } extern void *kvrealloc(const void *p, size_t oldsize, size_t newsize, gfp_t flags) - __alloc_size(3); + __realloc_size(3); extern void kvfree(const void *addr); extern void kvfree_sensitive(const void *addr, size_t len);
The __malloc attribute should not be applied to "realloc" functions, as the returned pointer may alias the storage of the prior pointer. Instead of splitting __malloc from __alloc_size, which would be a huge amount of churn, just create __realloc_size for the few cases where it is needed. Additionally removes the conditional test for __alloc_size__, which is always defined now. Cc: Pekka Enberg <penberg@kernel.org> Cc: David Rientjes <rientjes@google.com> Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Nick Desaulniers <ndesaulniers@google.com> Cc: Hao Luo <haoluo@google.com> Cc: Marco Elver <elver@google.com> Cc: linux-mm@kvack.org Signed-off-by: Kees Cook <keescook@chromium.org> --- include/linux/compiler_types.h | 13 +++++-------- include/linux/slab.h | 12 ++++++------ 2 files changed, 11 insertions(+), 14 deletions(-)