linux-ext4 - Re: [e2fsprogs PATCH] resize2fs: use directio when reading superblock

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: <20230817003705.GB1980@templeofstupid.com>
Date:   Wed, 16 Aug 2023 17:37:05 -0700
From:   Krister Johansen <kjlx@...pleofstupid.com>
To:     Theodore Ts'o <tytso@....edu>
Cc:     Krister Johansen <kjlx@...pleofstupid.com>,
        linux-ext4@...r.kernel.org
Subject: Re: [e2fsprogs PATCH] resize2fs: use directio when reading superblock

Hi Ted,

On Wed, Jun 28, 2023 at 04:02:20PM -0700, Krister Johansen wrote:
> Hi Ted,
> 
> On Fri, Jun 09, 2023 at 07:11:31PM -0700, Krister Johansen wrote:
> > On Fri, Jun 09, 2023 at 12:22:39AM -0400, Theodore Ts'o wrote:
> > > As far as your patch is concerned, resize2fs can do both off-line
> > > (unmounted) and on-line (mounted) resizes.  And turning direct I/O
> > > unconditionally isn't a great idea for off-line resizes --- it will
> > > really trash the performance of the resize.
> > 
> > Thanks for the additional detail.
> > 
> > I also double-checked to make sure these systems had the following patch
> > applied:
> > 
> > 05c2c00f3769 ext4: protect superblock modifications with a buffer lock
> > 
> > And they do.  Not sure if that's directly applicable to the online
> > resize case though.
> > 
> > > Does this patch work for you instead?
> > 
> > Thanks, it does!
> > 
> > > diff --git a/resize/main.c b/resize/main.c
> > > index 94f5ec6d..f914c050 100644
> > > --- a/resize/main.c
> > > +++ b/resize/main.c
> > > @@ -409,6 +409,8 @@ int main (int argc, char ** argv)
> > >  
> > >  	if (!(mount_flags & EXT2_MF_MOUNTED) && !print_min_size)
> > >  		io_flags = EXT2_FLAG_RW | EXT2_FLAG_EXCLUSIVE;
> > > +	if (mount_flags & EXT2_MF_MOUNTED)
> > > +		io_flags |= EXT2_FLAG_DIRECT_IO;
> > >  
> > >  	io_flags |= EXT2_FLAG_64BITS | EXT2_FLAG_THREADS;
> > >  	if (undo_file) {
> > 
> > If it counts:
> > 
> > Reviewed-by: Krister Johansen <kjlx@...pleofstupid.com>
> > Tested-by: Krister Johansen <kjlx@...pleofstupid.com>
> 
> Just wanted to check back on this.  Should I send a v2 that incorporates
> the changes you suggested above?

I've been running this patch on our production systems for the past
couple of months and haven't had any re-occurence of the bad superblock
checksum error.  It used to occur a few times a day.

Is there anything more I can do to help get this accepted into
e2fsprogs?

Thanks very much,

-K