linux-ext4 - Re: [PATCH v2 1/8] migrate: fix skipping metadata buffer heads on migration

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: <6qzm6z64hcrhbapzekg2eil5poi34tngghrumwoiaz2x25ogce@gatjy6egc2n3>
Date: Tue, 15 Apr 2025 13:17:28 +0200
From: Jan Kara <jack@...e.cz>
To: Luis Chamberlain <mcgrof@...nel.org>
Cc: Jan Kara <jack@...e.cz>, brauner@...nel.org, tytso@....edu, 
	adilger.kernel@...ger.ca, linux-ext4@...r.kernel.org, riel@...riel.com, dave@...olabs.net, 
	willy@...radead.org, hannes@...xchg.org, oliver.sang@...el.com, david@...hat.com, 
	axboe@...nel.dk, hare@...e.de, david@...morbit.com, djwong@...nel.org, 
	ritesh.list@...il.com, linux-fsdevel@...r.kernel.org, linux-block@...r.kernel.org, 
	linux-mm@...ck.org, gost.dev@...sung.com, p.raghav@...sung.com, da.gomez@...sung.com, 
	syzbot+f3c6fda1297c748a7076@...kaller.appspotmail.com
Subject: Re: [PATCH v2 1/8] migrate: fix skipping metadata buffer heads on
 migration

On Mon 14-04-25 15:19:33, Luis Chamberlain wrote:
> On Mon, Apr 14, 2025 at 02:09:46PM -0700, Luis Chamberlain wrote:
> > On Thu, Apr 10, 2025 at 02:05:38PM +0200, Jan Kara wrote:
> > > > @@ -859,12 +862,12 @@ static int __buffer_migrate_folio(struct address_space *mapping,
> > > >  			}
> > > >  			bh = bh->b_this_page;
> > > >  		} while (bh != head);
> > > > +		spin_unlock(&mapping->i_private_lock);
> > > 
> > > No, you've just broken all simple filesystems (like ext2) with this patch.
> > > You can reduce the spinlock critical section only after providing
> > > alternative way to protect them from migration. So this should probably
> > > happen at the end of the series.
> > 
> > So you're OK with this spin lock move with the other series in place?
> > 
> > And so we punt the hard-to-reproduce corruption issue as future work
> > to do? Becuase the other alternative for now is to just disable
> > migration for jbd2:
> > 
> > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> > index 1dc09ed5d403..ef1c3ef68877 100644
> > --- a/fs/ext4/inode.c
> > +++ b/fs/ext4/inode.c
> > @@ -3631,7 +3631,6 @@ static const struct address_space_operations ext4_journalled_aops = {
> >  	.bmap			= ext4_bmap,
> >  	.invalidate_folio	= ext4_journalled_invalidate_folio,
> >  	.release_folio		= ext4_release_folio,
> > -	.migrate_folio		= buffer_migrate_folio_norefs,
> >  	.is_partially_uptodate  = block_is_partially_uptodate,
> >  	.error_remove_folio	= generic_error_remove_folio,
> >  	.swap_activate		= ext4_iomap_swap_activate,
> 
> BTW I ask because.. are your expectations that the next v3 series also
> be a target for Linus tree as part of a fix for this spinlock
> replacement?

I have no problem with this (the use of data=journal mode is rare) but I
suspect this won't fix the corruption you're seeing because that happens in
the block device mapping. So to fix that you'd have to disable page
migration for block device mappings (i.e., do the above with def_blk_aops)
and *that* will make a lot of people unhappy.

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