[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Bug#760275: e2fsprogs: e2fsck corrupts Hurd filesystems



Hello,

Theodore Ts'o, on Wed 30 Dec 2015 23:56:46 -0500, wrote:
> On Thu, Jan 22, 2015 at 07:41:10PM +0100, Justus Winter wrote:
> > > 
> > > > since we have a bunch of workarounds
> > > > keyed off of that, and your patch will add another --- and I'm
> > > > planning on adding still more to make sure a user doesn't accidentally
> > > > turn on some ext4 features via mke2fs or tune2fs that use fields that
> > > > the Hurd is using for its own nefarious purposes.  Otherwise, the
> > > > usage clash would lead to no good....
> > > 
> > > If an e2fsprog or Linux uses the Linux specific stuff of the OS
> > > dependent fields without verifying the creator OS, then this is a bug.
> > > Fixing such a bug is no workaround.
> 
> I've been taking a closer look at the patch that you submitted to fix
> this bug (in the tree as 36769c606c2700) since it has been reported
> that this is causing a regression on FreeBSD.

I guess they need an || fs->super->s_creator_os == EXT2_OS_FREEBSD ?

> Taking a look at it more closely --- if this patch helps, this implies
> that Hurd users are turning on the ext4 HUGE_FILE feature,

?? No we definitely aren't.

> since we only do this test if the HUGE_FILE feature is enabled.

Please check the code again:

        if (!(fs->super->s_feature_incompat &
             EXT4_FEATURE_INCOMPAT_64BIT) &&

This triggers when the feature is *not* enabled, and just clears the
l_i_file_acl_high bit there without first checking that it's actually a
linuxish ext2.

> a) Is the hurd properly checking the ext4 feature flags?

AFAIK, yes.

> b) Why are Hurd users using file systems with the HUGE_FILE feature
> set?

Again, we don't.  We never added anything that does so.  If anything
does so, then it's a bug that was introduced, and most probably not in
our code since we just use mke2fs, we don't create FS ourselves.

BTW, about the bit in question:

“did the hurd developer who started using this field realize he has
foreclosed the use of file systems larger than 16TB on the hurd?”

At the time h_i_mode_high was added to the hurdish version of ext2, the
l_i_file_acl_high field was not even *defined* yet. So no, they didn't
realize that since that wasn't even existing yet.

> Clearly the Hurd ext2 file system developers can't be trusted to DTRT,

With the code sampled above, there is nothing done wrongly.  Please do
not make any such strong statement before things are actually checked as
being done wrongly.

Samuel


Reply to: