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: <aVIoyh9fXoxKTUSa@hyeyoo>
Date: Mon, 29 Dec 2025 16:07:54 +0900
From: Harry Yoo <harry.yoo@...cle.com>
To: Hao Li <hao.li@...ux.dev>
Cc: akpm@...ux-foundation.org, vbabka@...e.cz, andreyknvl@...il.com,
        cl@...two.org, dvyukov@...gle.com, glider@...gle.com,
        hannes@...xchg.org, linux-mm@...ck.org, mhocko@...nel.org,
        muchun.song@...ux.dev, rientjes@...gle.com, roman.gushchin@...ux.dev,
        ryabinin.a.a@...il.com, shakeel.butt@...ux.dev, surenb@...gle.com,
        vincenzo.frascino@....com, yeoreum.yun@....com, tytso@....edu,
        adilger.kernel@...ger.ca, linux-ext4@...r.kernel.org,
        linux-kernel@...r.kernel.org, cgroups@...r.kernel.org
Subject: Re: [PATCH] slub: clarify object field layout comments

On Wed, Dec 24, 2025 at 08:51:14PM +0800, Hao Li wrote:
> The comments above check_pad_bytes() document the field layout of a
> single object. Rewrite them to improve clarity and precision.
> 
> Also update an outdated comment in calculate_sizes().
> 
> Suggested-by: Harry Yoo <harry.yoo@...cle.com>
> Signed-off-by: Hao Li <hao.li@...ux.dev>
> ---
> Hi Harry, this patch adds more detailed object layout documentation. Let
> me know if you have any comments.

Hi Hao, thanks for improving it!
It looks much clearer now.

few nits below.

> + * Object field layout:
> + *
> + * [Left redzone padding] (if SLAB_RED_ZONE)
> + *   - Field size: s->red_left_pad
> + *   - Filled with 0xbb (SLUB_RED_INACTIVE) for inactive objects and
> + *     0xcc (SLUB_RED_ACTIVE) for objects in use when SLAB_RED_ZONE.

nit: although it becomes clear after reading the Notes: section,
I would like to make it clear that object address starts here (after
the left redzone) and the left redzone is right before each object.

> + * [Object bytes]
> + *   - Field size: s->object_size
> + *   - Object payload bytes.
> + *   - If the freepointer may overlap the object, it is stored inside
> + *     the object (typically near the middle).
> + *   - Poisoning uses 0x6b (POISON_FREE) and the last byte is
> + *     0xa5 (POISON_END) when __OBJECT_POISON is enabled.
> + *
> + * [Word-align padding] (right redzone when SLAB_RED_ZONE is set)
> + *   - Field size: s->inuse - s->object_size
> + *   - If redzoning is enabled and ALIGN(size, sizeof(void *)) adds no
> + *     padding, explicitly extend by one word so the right redzone is
> + *     non-empty.
> + *   - Filled with 0xbb (SLUB_RED_INACTIVE) for inactive objects and
> + *     0xcc (SLUB_RED_ACTIVE) for objects in use when SLAB_RED_ZONE.
> + *
> + * [Metadata starts at object + s->inuse]
> + *   - A. freelist pointer (if freeptr_outside_object)
> + *   - B. alloc tracking (SLAB_STORE_USER)
> + *   - C. free tracking (SLAB_STORE_USER)
> + *   - D. original request size (SLAB_KMALLOC && SLAB_STORE_USER)
> + *   - E. KASAN metadata (if enabled)
> + *
> + * [Mandatory padding] (if CONFIG_SLUB_DEBUG && SLAB_RED_ZONE)
> + *   - One mandatory debug word to guarantee a minimum poisoned gap
> + *     between metadata and the next object, independent of alignment.
> + *   - Filled with 0x5a (POISON_INUSE) when SLAB_POISON is set.
>
> + * [Final alignment padding]
> + *   - Any bytes added by ALIGN(size, s->align) to reach s->size.
> + *   - Filled with 0x5a (POISON_INUSE) when SLAB_POISON is set.
> + *
> + * Notes:
> + * - Redzones are filled by init_object() with SLUB_RED_ACTIVE/INACTIVE.
> + * - Object contents are poisoned with POISON_FREE/END when __OBJECT_POISON.
> + * - The trailing padding is pre-filled with POISON_INUSE by
> + *   setup_slab_debug() when SLAB_POISON is set, and is validated by
> + *   check_pad_bytes().
> + * - The first object pointer is slab_address(slab) +
> + *   (s->red_left_pad if redzoning); subsequent objects are reached by
> + *   adding s->size each time.
> + *
> + * If slabcaches are merged then the object_size and inuse boundaries are
> + * mostly ignored. Therefore no slab options that rely on these boundaries
>   * may be used with merged slabcaches.

For the last paragraph, perhaps it'll be clearer to say:

  "If a slab cache flag relies on specific metadata to exist at a fixed
   offset, the flag must be included in SLAB_NEVER_MERGE to prevent
   merging. Otherwise, the cache would misbehave as s->object_size and
   s->inuse are adjusted during cache merging"

Otherwise looks great to me, so please feel free to add:
Acked-by: Harry Yoo <harry.yoo@...cle.com>

-- 
Cheers,
Harry / Hyeonggon

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ