linux-kernel - Re: [PATCH] ext4: pass strlen() of the symlink instead of i_size to inode_set_cached

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: <vci2ejpu7eirvku6eg5ajrbsdlpztu2wgvm2n75lkiaenuxw7p@7ag5gflkjhus>
Date: Wed, 5 Feb 2025 19:10:49 +0100
From: Jan Kara <jack@...e.cz>
To: Mateusz Guzik <mjguzik@...il.com>
Cc: "Darrick J. Wong" <djwong@...nel.org>, brauner@...nel.org, 
	tytso@....edu, kees@...nel.org, viro@...iv.linux.org.uk, jack@...e.cz, 
	linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org, 
	syzbot+48a99e426f29859818c0@...kaller.appspotmail.com, linux-ext4 <linux-ext4@...r.kernel.org>
Subject: Re: [PATCH] ext4: pass strlen() of the symlink instead of i_size to
 inode_set_cached_link()

On Wed 05-02-25 18:33:23, Mateusz Guzik wrote:
> On Wed, Feb 5, 2025 at 6:29 PM Darrick J. Wong <djwong@...nel.org> wrote:
> >
> > On Wed, Feb 05, 2025 at 05:28:19PM +0100, Mateusz Guzik wrote:
> > > The call to nd_terminate_link() clamps the size to min(i_size,
> > > sizeof(ei->i_data) - 1), while the subsequent call to
> > > inode_set_cached_link() fails the possible update.
> > >
> > > The kernel used to always strlen(), so do it now as well.
> > >
> > > Reported-by: syzbot+48a99e426f29859818c0@...kaller.appspotmail.com
> > > Fixes: bae80473f7b0 ("ext4: use inode_set_cached_link()")
> > > Signed-off-by: Mateusz Guzik <mjguzik@...il.com>
> > > ---
> > >
> > > Per my comments in:
> > > https://lore.kernel.org/all/CAGudoHEv+Diti3r0x9VmF5ixgRVKk4trYnX_skVJNkQoTMaDHg@mail.gmail.com/#t
> > >
> > > There is definitely a pre-existing bug in ext4 which the above happens
> > > to run into. I suspect the nd_terminate_link thing will disappear once
> > > that gets sorted out.
> > >
> > > In the meantime the appropriate fix for 6.14 is to restore the original
> > > behavior of issuing strlen.
> > >
> > > syzbot verified the issue is fixed:
> > > https://lore.kernel.org/linux-hardening/67a381a3.050a0220.50516.0077.GAE@google.com/T/#m340e6b52b9547ac85471a1da5980fe0a67c790ac
> >
> > Again, this is evidence of inconsistent inode metadata, which should be
> > dealt with by returning EFSCORRUPTED, not arbitrarily truncating the
> > contents of a bad inode.
> >
> 
> I agree, rejecting the inode was something I was advocating for from the get go.
> 
> I don't know if a real patch(tm) will materialize for 6.14, so in the
> meantime I can at least damage-control this back to the original
> state.
> 
> If the ext4 folk do the right fix, I will be delighted to have this
> patch dropped. :)

Yeah, let me cook up proper ext4 fix for this (currently under testing).

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