On Fri, Mar 7, 2025 at 5:26 PM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:
>
> On Fri, Mar 07, 2025 at 05:11:55PM +0100, Mateusz Guzik wrote:
> > +++ b/include/linux/fs.h
> > @@ -2765,11 +2765,19 @@ struct audit_names;
> > struct filename {
> > const char *name; /* pointer to actual string */
> > const __user char *uptr; /* original userland pointer */
> > - atomic_t refcnt;
> > + union {
> > + atomic_t refcnt_atomic;
> > + int refcnt;
> > + };
> > +#ifdef CONFIG_DEBUG_VFS
> > + struct task_struct *owner;
> > +#endif
> > + bool is_atomic;
> > struct audit_names *aname;
> > const char iname[];
> > };
>
> 7 (or 3) byte hole; try to pad.
>
> Would it make more sense to put the bool between aname and iname where
> it will only take one byte instead of 8?
On the stock kernel there is already a 4 byte hole between the
refcount and aname, which is where is_atomic lands with debug
disabled. I.e. no size changes in production kernels with and without
the change.
However, now that you mention it the debug owner field is misplaced --
it should have landed *after* is_atomic. Maybe Christian will be happy
to just move it, otherwise I'm going to include this in a v2.
The iname field is expected to be aligned, so I don't believe
shuffling the is_atomic flag helps anyone:
static_assert(offsetof(struct filename, iname) % sizeof(long) == 0);
--
Mateusz Guzik <mjguzik gmail.com>
