linux-kernel - Re: [PATCH V4 8/8] mm/slab: place slabobj_ext metadata in unused space within s->size

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: <aVNcTVKmz9N6bOfF@hyeyoo>
Date: Tue, 30 Dec 2025 13:59:57 +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 V4 8/8] mm/slab: place slabobj_ext metadata in unused
 space within s->size

On Wed, Dec 24, 2025 at 08:43:17PM +0800, Hao Li wrote:
> On Wed, Dec 24, 2025 at 03:38:57PM +0900, Harry Yoo wrote:
> > On Wed, Dec 24, 2025 at 01:33:59PM +0800, Hao Li wrote:
> > > One more thought: in calculate_sizes() we add some extra padding when
> > > SLAB_RED_ZONE is enabled:
> > > 
> > > if (flags & SLAB_RED_ZONE) {
> > > 	/*
> > > 	 * Add some empty padding so that we can catch
> > > 	 * overwrites from earlier objects rather than let
> > > 	 * tracking information or the free pointer be
> > > 	 * corrupted if a user writes before the start
> > > 	 * of the object.
> > > 	 */
> > > 	size += sizeof(void *);
> > > 	...
> > > }
> > > 
> > > 
> > > From what I understand, this additional padding ends up being placed
> > > after the KASAN allocation metadata.
> > 
> > Right.
> > 
> > > Since it’s only "extra" padding (i.e., it doesn’t seem strictly required
> > > for the layout), and your patch would reuse this area — together with
> > > the final padding introduced by `size = ALIGN(size, s->align);`
> > 
> > Very good point!
> > Nah, it wasn't intentional to reuse the extra padding.

Waaaait, now I'm looking into it again to write V5...

It may reduce (or remove) the space for the final padding but not the
mandatory padding because the mandatory padding is already included
in the size before `aligned_size = ALIGN(size, s->align)`

> > > for objext, it seems like this padding may no longer provide much benefit.
> > > Do you think it would make sense to remove this extra padding
> > > altogether?
> > 
> > I think when debugging flags are enabled it'd still be useful to have,
> 
> Absolutely — I’m with you on this.
> 
> After thinking about it again, I agree it’s better to keep it.
> 
> Without that mandatory extra word, we could end up with "no trailing
> padding at all" in cases where ALIGN(size, s->align) doesn’t actually
> add any bytes.
> 
> > I'll try to keep the padding area after obj_ext (so that overwrites from
> > the previous object won't overwrite the metadata).
> 
> Agree — we should make sure there is at least sizeof(void *) of extra
> space after obj_exts when SLAB_RED_ZONE is enabled, so POISON_INUSE has
> somewhere to go.

I think V4 of the patchset is already doing that, no?

The mandatory padding exists after obj_ext if SLAB_RED_ZONE is enabled
and the final padding may or may not exist. check_pad_bytes() already knows
that the padding(s) exist after obj_ext.

By the way, thanks for fixing the comment once again,
it's easier to think about the layout now.

-- 
Cheers,
Harry / Hyeonggon