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: <aPKilSNCQRW9c6rl@cern.ch>
Date: Fri, 17 Oct 2025 15:09:57 -0500
From: Dave Dykstra <dwd@...n.ch>
To: "Darrick J. Wong" <djwong@...nel.org>
Cc: linux-ext4@...r.kernel.org,
	Dave Dykstra <2129743+DrDaveD@...rs.noreply.github.com>
Subject: Re: [PATCH] fuse2fs: updates for message reporting journal is not
 supported

On Fri, Oct 17, 2025 at 12:18:00PM -0700, Darrick J. Wong wrote:
> On Thu, Oct 16, 2025 at 03:09:03PM -0500, Dave Dykstra wrote:
> > This makes two changes to the message that is shown saying that fuse2fs
> > does not support the journal.  First is that it reverts the check to
> > what it was before 3875380 to look at the ro option not being set
> > instead of checking the RW flag.  That's because I don't think this
> > message needs to be shown when the ro option is set even when it was
> > opened RW; there should be nothing to corrupt when it is ro.
> > 
> > Second, it changes the message to say that writing is not supported
> > rather than using the journal is not supported.  The current message is
> > confusing because in fact the journal is used for recovery when needed
> > and possible.
> > 
> > Also submitted as PR https://github.com/tytso/e2fsprogs/pull/251
> > 
> > Signed-off-by: Dave Dykstra <2129743+DrDaveD@...rs.noreply.github.com>
> > ---
> >  misc/fuse2fs.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/misc/fuse2fs.c b/misc/fuse2fs.c
> > index cb5620c7..c46cc03b 100644
> > --- a/misc/fuse2fs.c
> > +++ b/misc/fuse2fs.c
> > @@ -4774,10 +4774,10 @@ int main(int argc, char *argv[])
> >  		}
> >  	}
> >  
> > -	if (global_fs->flags & EXT2_FLAG_RW) {
> > +	if (!fctx.ro) {
> 
> Again, rw != EXT2_FLAG_RW.
> 
> The ro and rw mount options specify if the filesystem mount is writable.
> You can mount a filesystem in multiple places, and some of the mounts
> can be ro and some can be rw.
> 
> EXT2_FLAG_RW specifies that the filesystem driver can write to the block
> device.  fuse2fs should warn about incomplete journal support any time
> the **filesystem** is writable, independent of the write state of the
> mount.

Are you saying that is indeed possible for a read-only mount to cause
file corruption or data loss if there's not a graceful unmount?  If so,
it sure seems like that should be avoided if possible!  Since fuse2fs
does not support writing the journal, perhaps its behavior should be
different than the kernel's behavior for this too.  Perhaps once the
journal is recovered it should be remounted without EXT2_FLAG_RW.

In any case, during further testing I did find a serious problem with
this change in that it changes more than just the message; it also skips
reading in the inode bitmap, which causes a problem later.  So at
minimum this patch should only affect the message, not the rest of the
stuff in that if statement.

> Filesystems are allowed to write to the block device even if the mount
> itself is readonly, e.g. kernel ext4 recovering the journal on an ro
> mount.
> 
> NAK.
> 
> --D
> 
> >  		if (ext2fs_has_feature_journal(global_fs->super))
> >  			log_printf(&fctx, "%s",
> > - _("Warning: fuse2fs does not support using the journal.\n"
> > + _("Warning: fuse2fs does not support writing the journal.\n"

What do you think about this message change?

Dave

> >     "There may be file system corruption or data loss if\n"
> >     "the file system is not gracefully unmounted.\n"));
> >  		err = ext2fs_read_inode_bitmap(global_fs);
> > -- 
> > 2.43.5
> > 
> > 
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ