Re: [RFC PATCH] slab: don't assume alignment on allocators that may return ZERO_SIZE_PTR

[Harry Yoo <harry@kernel.org>]

[-- Attachment #1.1: Type: text/plain, Size: 6520 bytes --]


[+Cc Catalin for ARCH_KMALLOC_MINALIGN bits]

On 7/12/26 9:07 PM, Karl Mehltretter wrote:
> __kmalloc_noprof(), __kmalloc_node_noprof(), and __kmalloc_flags_noprof()
> are annotated with __assume_kmalloc_alignment, which expands to
> __assume_aligned(ARCH_KMALLOC_MINALIGN). All three can return
> ZERO_SIZE_PTR, currently (void *)16, for zero-sized requests. When
> ARCH_KMALLOC_MINALIGN exceeds 16, this contradicts the annotation.

Ouch.

> Compilers may use this false assumption to reduce ZERO_OR_NULL_PTR() to a
> NULL check, losing recognition of ZERO_SIZE_PTR. Current GCC and Clang
> retain the existing range check through kmalloc's inline wrapper, so no
> functional miscompile was reproduced with the current source form. However,
> both eliminate an exact ZERO_SIZE_PTR comparison on the same return value.
> With Clang, UBSAN also reports the invalid alignment assumption at boot.
> 
> Changing ZERO_SIZE_PTR to satisfy the annotation would make its value and
> the range accepted by ZERO_OR_NULL_PTR() architecture-dependent. Avoid that
> semantic change by dropping the annotation from the general kmalloc entry
> points. Retain it on the cache helpers, which cannot return the sentinel.
> Allocation behavior is unchanged.
> 
> Kernels before v7.2 do not have __kmalloc_flags_noprof().
> Backports to those kernels only need the include/linux/slab.h change.
> 
> Fixes: 94a58c360a45 ("slab.h: sprinkle __assume_aligned attributes")
> Fixes: f6d50ab29afd ("mm/slab: introduce kmalloc_flags()")
> Cc: <stable@vger.kernel.org> # needs adjustment before v7.2
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
> ---
> 
> Notes:
>     This is RFC because the alignment contract can be corrected in three ways:

I think it's still worth having at least a weaker alignment guarantee
and would like to vote for option 3 (or option 2, if 3 turns out to be
infeasible) unless there's something unexpected that prevents us from
doing that.

>       1. Remove the annotation from entry points that can return ZERO_SIZE_PTR
>          (this patch). On armv5 this increases .text by 1120 bytes (0.02%);
>          no measurable change was seen on arm64.
>     
>       2. Cap the assumed alignment at 16. This preserves some alignment
>          information but couples the annotation to the current sentinel value.
>
>       3. Change ZERO_SIZE_PTR to satisfy ARCH_KMALLOC_MINALIGN. This makes the
>          long-standing sentinel architecture-dependent and either expands the
>          range accepted by ZERO_OR_NULL_PTR() or requires changing that macro.

Some code (e.g., kmem_dump_obj()) performs (addr < PAGE_SIZE) check
either NULL or ZERO_SIZE_PTR) to see if the address is valid.

Bumping ZERO_SIZE_PTR to something smaller than PAGE_SIZE should still
work: (addr < PAGE_SIZE) check still works, and accessing it still
causes a fault. No arch should have ARCH_KMALLOC_MINALIGN >= PAGE_SIZE?

-- 
Cheers,
Harry / Hyeonggon


>     On armv5, Clang UBSAN reports the invalid alignment assumption during boot;
>     the report disappears with this patch.
>     
>     Clang 22 and GCC 13 retain ZERO_OR_NULL_PTR()'s existing range check through
>     _kmalloc_noprof(), so no functional miscompile was reproduced in current
>     code. This is an optimizer limitation, not a guarantee. For a kmalloc return
>     value, writing the check as "!p || p == ZERO_SIZE_PTR" causes both compilers
>     to remove the ZERO_SIZE_PTR comparison. Built into an armv5 kernel with that
>     form, a zero-size allocation is then no longer recognised and is dereferenced:
>     
>       Unable to handle kernel NULL pointer dereference at virtual address 00000010
>       Internal error: Oops: 805 [#1] ARM
>       PC is at __fc_init+0x3c/0x60
>       Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
>     
>     Without the annotation the comparison is retained and the kernel boots.
>     
>     Would the slab maintainers prefer removing the annotation or retaining a
>     weaker, valid alignment guarantee?
>
>  include/linux/slab.h | 12 ++++++++----
>  mm/slab.h            |  3 ++-
>  2 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index 51f03f18c9a7..1c63048f6467 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -632,8 +632,12 @@ static inline unsigned int arch_slab_minalign(void)
>  
>  /*
>   * kmem_cache_alloc and friends return pointers aligned to ARCH_SLAB_MINALIGN.
> - * kmalloc and friends return pointers aligned to both ARCH_KMALLOC_MINALIGN
> - * and ARCH_SLAB_MINALIGN, but here we only assume the former alignment.
> + * Objects allocated by kmalloc and friends are aligned to both
> + * ARCH_KMALLOC_MINALIGN and ARCH_SLAB_MINALIGN, but here we only assume the
> + * former alignment.
> + *
> + * This guarantee does not apply to ZERO_SIZE_PTR, which is not an allocated
> + * object.
>   */
>  #define __assume_kmalloc_alignment __assume_aligned(ARCH_KMALLOC_MINALIGN)
>  #define __assume_slab_alignment __assume_aligned(ARCH_SLAB_MINALIGN)
> @@ -939,10 +943,10 @@ unsigned int kmem_cache_sheaf_size(struct slab_sheaf *sheaf);
>   */
>  
>  void *__kmalloc_noprof(DECL_TOKEN_PARAMS(size, token), gfp_t flags)
> -				__assume_kmalloc_alignment __alloc_size(1);
> +				__alloc_size(1);
>  
>  void *__kmalloc_node_noprof(DECL_KMALLOC_PARAMS(size, b, token), gfp_t flags, int node)
> -				__assume_kmalloc_alignment __alloc_size(1);
> +				__alloc_size(1);
>  
>  void *__kmalloc_cache_noprof(struct kmem_cache *s, gfp_t flags, size_t size)
>  				__assume_kmalloc_alignment __alloc_size(3);
> diff --git a/mm/slab.h b/mm/slab.h
> index 281a65233795..aea1373c7f83 100644
> --- a/mm/slab.h
> +++ b/mm/slab.h
> @@ -28,9 +28,10 @@ static inline bool alloc_flags_allow_spinning(const unsigned int alloc_flags)
>  	return !(alloc_flags & SLAB_ALLOC_NOLOCK);
>  }
>  
> +/* Can return ZERO_SIZE_PTR, so not tagged __assume_kmalloc_alignment. */
>  void *__kmalloc_flags_noprof(DECL_TOKEN_PARAMS(size, token), gfp_t flags,
>  				  unsigned int alloc_flags, int node)
> -				  __assume_kmalloc_alignment __alloc_size(1);
> +				  __alloc_size(1);
>  
>  static __always_inline __alloc_size(1) void *_kmalloc_flags_noprof(size_t size,
>  		gfp_t flags, unsigned int alloc_flags, int node, kmalloc_token_t token)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]