linux-ext4 - Re: generic_permission() optimization

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] [day] [month] [year] [list]

Message-ID: <kn44smk4dgaj5rqmtcfr7ruecixzrik6omur2l2opitn7lbvfm@rm4y24fcfzbz>
Date: Wed, 5 Nov 2025 14:37:05 +0100
From: Jan Kara <jack@...e.cz>
To: Mateusz Guzik <mjguzik@...il.com>
Cc: Theodore Ts'o <tytso@....edu>, 
	Linus Torvalds <torvalds@...ux-foundation.org>, Christian Brauner <brauner@...nel.org>, 
	Al Viro <viro@...iv.linux.org.uk>, linux-fsdevel <linux-fsdevel@...r.kernel.org>, 
	Jan Kara <jack@...e.cz>, Ext4 Developers List <linux-ext4@...r.kernel.org>
Subject: Re: generic_permission() optimization

On Wed 05-11-25 12:51:16, Mateusz Guzik wrote:
> On Wed, Nov 5, 2025 at 12:50 PM Mateusz Guzik <mjguzik@...il.com> wrote:
> >
> > On Sun, Apr 13, 2025 at 2:40 PM Theodore Ts'o <tytso@....edu> wrote:
> > >
> > > On Sun, Apr 13, 2025 at 11:41:47AM +0200, Mateusz Guzik wrote:
> > > > This is the rootfs of the thing, so I tried it out with merely
> > > > printing it. I got 70 entries at boot time. I don't think figuring out
> > > > what this is specifically is warranted (it is on debian though).
> > >
> > > Well, can you run:
> > >
> > > debugfs -R "stat <INO>" /dev/ROOT_DEV
> > >
> > > on say, two or three of the inodes (replace INO with a number, and
> > > ROOT_DEV with the root file system device) and send me the result?
> > > That would be really helpful in understanding what might be going on.
> > >
> > > > So... I think this is good enough to commit? I had no part in writing
> > > > the patch and I'm not an ext4 person, so I'm not submitting it myself.
> > > >
> > > > Ted, you seem fine with the patch, so perhaps you could do the needful(tm)?
> > >
> > > Sure, I'll put together a more formal patch and do full QA run and
> > > checking of the code paths, as a supposed a fairly superficial review
> > > and hack.
> > >
> >
> > It looks like this well through the cracks.
> >
> > To recount, here is the patch (by Linus, not me):
> > > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> > > index f386de8c12f6..3e0ba7c4723a 100644
> > > --- a/fs/ext4/inode.c
> > > +++ b/fs/ext4/inode.c
> > > @@ -5109,6 +5109,11 @@ struct inode *__ext4_iget(struct super_block *sb, unsigned long ino,
> > >                 goto bad_inode;
> > >         brelse(iloc.bh);
> > >
> > > +       if (test_opt(sb, DEBUG) &&
> > > +           (ext4_test_inode_state(inode, EXT4_STATE_XATTR) ||
> > > +            ei->i_file_acl))
> > > +               ext4_msg(sb, KERN_DEBUG, "has xattr ino %lu", inode->i_ino);
> > > +
> > >         unlock_new_inode(inode);
> > >         return inode;
> >
> 
> sigh, copy-pasto, the patch is:
>   --- a/fs/ext4/inode.c
>   +++ b/fs/ext4/inode.c
>   @@ -5011,6 +5011,11 @@ struct inode *__ext4_iget(...
>         }
> 
>         brelse(iloc.bh);
>   +
>   +     /* Initialize the "no ACL's" state for the simple cases */
>   +     if (!ext4_test_inode_state(inode, EXT4_STATE_XATTR) && !ei->i_file_acl)
>   +             cache_no_acl(inode);
>   +
>         unlock_new_inode(inode);
>         return inode;

This looks fine. Feel free to add:

Reviewed-by: Jan Kara <jack@...e.cz>

								Honza
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR