Ted et al.
I've only just noticed this, so I have no idea how long it has been this way.
When I build a kernel with CONFIG_EXT4_USE_FOR_EXT23=y and boot from it,
the ext4 root filesystem shows up as "ext2" mode, rather than "ext4".
This looks very wrong to me, and quite dangerous.
Eg. I test it by building my own kernel (2.6.38.2), with ext4 built-in,
no initramfs required, and boot:
root=/dev/sda1 init=/bin/bash
...
$ mount /proc
$ cat /proc/mounts
/dev/root / ext2 ro,relatime,barrier=1,data=ordered 0 0
So.. it shows "ext2" instead of "ext4". That really looks like a bug.
Especially since it appears to be using journaling regardless.
Building the kernel without CONFIG_EXT4_USE_FOR_EXT23
results in a proper "ext4" mount entry in /proc/mounts.
On Tue, Apr 12, 2011 at 01:25:08PM -0400, Mark Lord wrote:
> Ted et al.
>
> I've only just noticed this, so I have no idea how long it has been this way.
>
> When I build a kernel with CONFIG_EXT4_USE_FOR_EXT23=y and boot from it,
> the ext4 root filesystem shows up as "ext2" mode, rather than "ext4".
>
> This looks very wrong to me, and quite dangerous.
It's a cosemtic bug, I agree, but I'm not sure why you consider it
dangerous.
CONFIG_EXT4_USE_FOR_EXT23 means that ext4 registers itself as ext2
and/or ext3, if ext2 and/or ext3 are not configured into the kernel.
Since the kernel tries to mount the file system as ext2, ext3, and
then ext4, and uses whichever one works first.
- Ted
On 11-04-12 08:49 PM, Ted Ts'o wrote:
>
> It's a cosemtic bug, I agree
Great. Got a patch to fix it for me to try?
Since it currently misidentifies the filesystem type,
some of the disk management tools I have get confused.
I don't like taking any chances with user data like that.
Thanks.
On 11-04-12 08:49 PM, Ted Ts'o wrote:
> On Tue, Apr 12, 2011 at 01:25:08PM -0400, Mark Lord wrote:
>> Ted et al.
>>
>> I've only just noticed this, so I have no idea how long it has been this way.
>>
>> When I build a kernel with CONFIG_EXT4_USE_FOR_EXT23=y and boot from it,
>> the ext4 root filesystem shows up as "ext2" mode, rather than "ext4".
>>
>> This looks very wrong to me, and quite dangerous.
>
> It's a cosemtic bug, I agree, but I'm not sure why you consider it
> dangerous.
It is dangerous if it really has mounted an ext4 as ext2;
the on-disk formats are not 100% compatible.
I don't know what it is doing, so it looks quite dangerous.
Perhaps it really did mount as ext4 though, in which case not.
Thanks.
>>>>> "Ted" == Ted Ts'o <[email protected]> writes:
Ted> On Tue, Apr 12, 2011 at 01:25:08PM -0400, Mark Lord wrote:
>> Ted et al.
>>
>> I've only just noticed this, so I have no idea how long it has been this way.
>>
>> When I build a kernel with CONFIG_EXT4_USE_FOR_EXT23=y and boot from it,
>> the ext4 root filesystem shows up as "ext2" mode, rather than "ext4".
>>
>> This looks very wrong to me, and quite dangerous.
Ted> It's a cosemtic bug, I agree, but I'm not sure why you consider it
Ted> dangerous.
Ted> CONFIG_EXT4_USE_FOR_EXT23 means that ext4 registers itself as
Ted> ext2 and/or ext3, if ext2 and/or ext3 are not configured into the
Ted> kernel. Since the kernel tries to mount the file system as ext2,
Ted> ext3, and then ext4, and uses whichever one works first.
Should it instead be: CONFIG_EXT4_MASQUERADE_AS_EXT23 instead, so
it's blindingly obvious what's going on here.
John
On 04/13/2011 12:45 PM, John Stoffel wrote:
>>>>>> "Ted" == Ted Ts'o<[email protected]> writes:
> Ted> On Tue, Apr 12, 2011 at 01:25:08PM -0400, Mark Lord wrote:
>>> Ted et al.
>>>
>>> I've only just noticed this, so I have no idea how long it has been this way.
>>>
>>> When I build a kernel with CONFIG_EXT4_USE_FOR_EXT23=y and boot from it,
>>> the ext4 root filesystem shows up as "ext2" mode, rather than "ext4".
>>>
>>> This looks very wrong to me, and quite dangerous.
> Ted> It's a cosemtic bug, I agree, but I'm not sure why you consider it
> Ted> dangerous.
>
> Ted> CONFIG_EXT4_USE_FOR_EXT23 means that ext4 registers itself as
> Ted> ext2 and/or ext3, if ext2 and/or ext3 are not configured into the
> Ted> kernel. Since the kernel tries to mount the file system as ext2,
> Ted> ext3, and then ext4, and uses whichever one works first.
>
> Should it instead be: CONFIG_EXT4_MASQUERADE_AS_EXT23 instead, so
> it's blindingly obvious what's going on here.
>
> John
No - the idea is to have use eliminate duplicate code and still be able to run
ext2 and ext3 in the same way.
How close we come to that goal will probably vary between ext2 and ext3 now but
it would be great to get that done eventually.
We did discuss this at LSF last week and we had some enthusiasm for keeping ext2
code around forever more or less as "the simple" file system template :)
Ric
On Apr 13, 2011, at 10:10 AM, Mark Lord wrote:
>
> It is dangerous if it really has mounted an ext4 as ext2;
> the on-disk formats are not 100% compatible.
>
> I don't know what it is doing, so it looks quite dangerous.
> Perhaps it really did mount as ext4 though, in which case not.
As the config option name: CONFIG_EXT4_USE_FOR_EXT23
might mave suggested to you, what this means is that we are
using the ext4 file system for ext2/3 file systems. So yes, we are
using the ext4 file system driver when the ext2 file system is
requested. Since the kernel tries mounting the "ext2" file system
first, since we are using the ext4 file system driver, it succeeds,
and since it was mounted when the requested name was "ext2",
that is what is recorded in the mount table.
It's not dangerous at all. Again, as you can tell if you were to actually
look at your .config carefully, the ext2 file system was not compiled
into your kernel at all. Ext4 will only try masquerading as ext2 if
CONFIG_EXT2_FS is disabled. So it couldn't have possibly been
using ext2 code, if you thought about it for a second.
I can write up a patch which explicitly tests for feature flags that go
beyond ext2 as of a particular version, and if so, refuse the mount
when ext4 is masquerading as ext2, and do the same for ext3. I
probably will do this to avoid user questions, when I have some
spare time.
However, please note that this will have no actual effect on how
anything int he kernel will behave --- none ---- except for a one
character change in /proc/mounts: s/2/4/. (This is because the
kernel will now try ext2, and fail, try ext3 and fail, and then succeed
when the exact same file system driver is used. Oh, it might also
extend the boot time by a few milliseconds.)
Have I made it clear enough now to assuage your fears?
-- Ted
On Wed, Apr 13, 2011 at 05:00:34PM -0400, Theodore Tso wrote:
> I can write up a patch which explicitly tests for feature flags that go
> beyond ext2 as of a particular version, and if so, refuse the mount
> when ext4 is masquerading as ext2, and do the same for ext3. I
> probably will do this to avoid user questions, when I have some
> spare time.
This is the correct behavior. Few people understand the
filesystem type test ordering, and fewer (these days) modify their own
.config. They expect that the name in /proc/mounts reflects the format
on the platter. If we say 'ext2', they think it's a non-journaled FFS.
Errors in the other direction are less confusing. If you wanted
a quick hack, you could just have ext4 always fail ext2/3 mounts and
report itself as ext4 no matter what the physical disk looks like.
People would understand that 'ext4' in /proc/mounts means that the ext4
driver has mounted an extN filesystem much faster than they would
understand that 'ext2' means the ext4 driver has mounted an ext4
filesystem but with the scanning name of ext2.
Joel
--
"If at first you don't succeed, cover all traces that you tried."
-Unknown
http://www.jlbec.org/
[email protected]
On 11-04-13 05:00 PM, Theodore Tso wrote:
>
> It's not dangerous at all. Again, as you can tell if you were to actually
> look at your .config carefully, the ext2 file system was not compiled
> into your kernel at all. Ext4 will only try masquerading as ext2 if
> CONFIG_EXT2_FS is disabled. So it couldn't have possibly been
> using ext2 code, if you thought about it for a second.
>
> I can write up a patch which explicitly tests for feature flags that go
> beyond ext2 as of a particular version, and if so, refuse the mount
> when ext4 is masquerading as ext2, and do the same for ext3. I
> probably will do this to avoid user questions, when I have some
> spare time.
>
> However, please note that this will have no actual effect on how
> anything int he kernel will behave --- none ---- except for a one
> character change in /proc/mounts: s/2/4/. (This is because the
> kernel will now try ext2, and fail, try ext3 and fail, and then succeed
> when the exact same file system driver is used. Oh, it might also
> extend the boot time by a few milliseconds.)
>
> Have I made it clear enough now to assuage your fears?
No, very much the contrary, despite the condescending
attempt at "explanation".
config EXT4_USE_FOR_EXT23
bool "Use ext4 for ext2/ext3 file systems"
depends on EXT4_FS
depends on EXT3_FS=n || EXT2_FS=n
default y
help
Allow the ext4 file system driver code to be used for ext2 or
ext3 file system mounts. This allows users to reduce their
compiled kernel size by using one file system driver for
ext2, ext3, and ext4 file systems.
So the option is supposed to allow ext4 code to be used
to access ext2 and ext3 filesystems, saving some space
in the binary kernel. That much is obvious.
But far far less obvious is why the kernel reports
having mounted my ext4 filesystem as "ext2",
and what exactly it means by that.
And I've been a kernel developer as long as you have, Ted,
so just imagine the confusion this might be causing lots
of other less experienced people.
Thanks.
On Apr 13, 2011, at 9:34 PM, Mark Lord wrote:
> But far far less obvious is why the kernel reports
> having mounted my ext4 filesystem as "ext2",
> and what exactly it means by that.
Because the kernel reports the type that it successfully matched against when the mount succeeded, and in order for ext4 to take over ext2, it calls register_filesystem() as ext2. Since the kernel (a) tries mounting the root file system as ext2 (it tries blindly until it succeeds), (b) when ext4 is taking care of ext2 mounts, an filesystem with ext4 features can be mounted by the ext4 file system driver, (c) the base VFS layer controls what types is listed, and since it succeeded when it tried mounting as ext2, it reports it as ext2.
In other words, what the kernel reports is not what file system driver is currently servicing your block device, but the name of the fs type first succeeded when the root file system mount did a brute-force search. (BTW, some distributions may use an initrd that uses a more sophisticated userspace fs probing logic that will correctly determine that the file system is really an ext4 file system, and explicitly request an ext4 mount, and then the kernel will report that it was mounted with an ext4 fs type.) The brute-force search nature of the kernel can potentially get you in trouble, since if a block device was originally formatted as a FAT file system, and then later reformatted as some other file system, such as btrfs, ext4, xfs, etc., in theory, depending on the order the kernel tri
es its brute force search, it could conceivably get things wrong. This is a long-known problem, and we mostly work around it by making the mkfs programs zero out enough data blocks that it's unlikely that a reused block device might get mistaken as something else.
I repeat; this is a cosmetic issue _only_. I suspect most users won't be looking that closely at what the kernel outputs, but I agree it's something that we should fix. OK?
-- Ted
The intent of CONFIG_EXT4_USE_FOR_EXT23 is to provide an alias so
that the ext4 module is sufficient even if user space calls out ext2.
However, it currently registers the alias names before registering ext4.
This means they show up first in /proc/filesystems and ext3 and ext4
formatted file systems show up in the mount list as ext4.
Register the filesystem aliases after ext4 so they show as ext4 until the
filesystems is filtered. This will avoid users or tools configuring a
kernel with just ext2 and wondering why there system broke. Also register
in the same order as listed in fs/Makefile.
Signed-off-by: Milton Miller <[email protected]>
---
Untested but simple code movement. Mark will you please test?
Is it safe to initialize ext4_li_info and ext4_li_mtx after we register
the filesystem? What keeps the init after the first call to mount
a filesystem?
I checked unregister_filesystem and it is safe when the file system
type is and is not registered.
Index: work.git/fs/ext4/super.c
===================================================================
--- work.git.orig/fs/ext4/super.c 2011-04-14 09:26:43.250066794 -0500
+++ work.git/fs/ext4/super.c 2011-04-14 09:29:53.220061521 -0500
@@ -4898,11 +4898,11 @@ static int __init ext4_init_fs(void)
err = init_inodecache();
if (err)
goto out1;
- register_as_ext2();
- register_as_ext3();
err = register_filesystem(&ext4_fs_type);
if (err)
goto out;
+ register_as_ext3();
+ register_as_ext2();
ext4_li_info = NULL;
mutex_init(&ext4_li_mtx);
On 11-04-14 11:41 AM, Milton Miller wrote:
> The intent of CONFIG_EXT4_USE_FOR_EXT23 is to provide an alias so
> that the ext4 module is sufficient even if user space calls out ext2.
> However, it currently registers the alias names before registering ext4.
> This means they show up first in /proc/filesystems and ext3 and ext4
> formatted file systems show up in the mount list as ext4.
Shouldn't that say:
This means they show up first in /proc/filesystems, causing ext3 and
ext4 formatted file systems to show up in the mount list as ext2.
Thanks. Other than that, it all looks good here with ext2 and ext4
file systems. I didn't try an ext3 filesystem, but with it being between
the other two, I expect it to also be fine.
> Register the filesystem aliases after ext4 so they show as ext4 until the
> filesystems is filtered. This will avoid users or tools configuring a
> kernel with just ext2 and wondering why there system broke. Also register
> in the same order as listed in fs/Makefile.
>
> Signed-off-by: Milton Miller <[email protected]>
> ---
>
> Untested but simple code movement. Mark will you please test?
>
> Is it safe to initialize ext4_li_info and ext4_li_mtx after we register
> the filesystem? What keeps the init after the first call to mount
> a filesystem?
>
> I checked unregister_filesystem and it is safe when the file system
> type is and is not registered.
>
> Index: work.git/fs/ext4/super.c
> ===================================================================
> --- work.git.orig/fs/ext4/super.c 2011-04-14 09:26:43.250066794 -0500
> +++ work.git/fs/ext4/super.c 2011-04-14 09:29:53.220061521 -0500
> @@ -4898,11 +4898,11 @@ static int __init ext4_init_fs(void)
> err = init_inodecache();
> if (err)
> goto out1;
> - register_as_ext2();
> - register_as_ext3();
> err = register_filesystem(&ext4_fs_type);
> if (err)
> goto out;
> + register_as_ext3();
> + register_as_ext2();
>
> ext4_li_info = NULL;
> mutex_init(&ext4_li_mtx);
On Thu, Apr 14, 2011 at 10:30:07AM -0500, Milton Miller wrote:
> The intent of CONFIG_EXT4_USE_FOR_EXT23 is to provide an alias so
> that the ext4 module is sufficient even if user space calls out ext2.
> However, it currently registers the alias names before registering ext4.
> This means they show up first in /proc/filesystems and ext3 and ext4
> formatted file systems show up in the mount list as ext4.
Note this is only true for the root file system.
>
> Register the filesystem aliases after ext4 so they show as ext4 until the
> filesystems is filtered. This will avoid users or tools configuring a
> kernel with just ext2 and wondering why there system broke. Also register
> in the same order as listed in fs/Makefile.
>
> Signed-off-by: Milton Miller <[email protected]>
This is actually the better way to fix Mark's complaint, as it
provides the correct behaviour if someone tries explicitly mounting an
ext4 file system via "mount -t ext2 /dev/sda". It also provides a
better explanation of what happens when CONFIG_EXT4_USE_EXT23 is
enabled and ext2 and ext3 are deconfigured, and the user has an ext2
or ext3 root file system. Now they will something like this:
EXT4-fs (sda): mounting ext2 file system using the ext4 subsystem
EXT4-fs (sda): mounted filesystem without journal. Opts: (null)
which makes clearer what is going on.
- Ted
>From 5a5baedb6306edb9effc86add73e9522e2a70532 Mon Sep 17 00:00:00 2001
From: Theodore Ts'o <[email protected]>
Date: Sat, 16 Apr 2011 21:31:57 -0400
Subject: [PATCH] ext4: check for ext[23] file system features when mounting as ext[23]
Provide better emulation for ext[23] mode by enforcing that the file
system does not have any unsupported file system features as defined
by ext[23] when emulating the ext[23] file system driver when
CONFIG_EXT4_USE_FOR_EXT23 is defined.
This causes the file system type information in /proc/mounts to be
correct for the automatically mounted root file system. This also
means that "mount -t ext2 /dev/sda /mnt" will fail if /dev/sda
contains an ext3 or ext4 file system, just as one would expect if the
original ext2 file system driver were in use.
Signed-off-by: "Theodore Ts'o" <[email protected]>
---
fs/ext4/ext4.h | 15 +++++++++++
fs/ext4/super.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++------
2 files changed, 79 insertions(+), 8 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 4daaf2b..076c5d2 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1351,6 +1351,21 @@ static inline void ext4_clear_state_flags(struct ext4_inode_info *ei)
#define EXT4_FEATURE_INCOMPAT_EA_INODE 0x0400 /* EA in inode */
#define EXT4_FEATURE_INCOMPAT_DIRDATA 0x1000 /* data in dirent */
+#define EXT2_FEATURE_COMPAT_SUPP EXT4_FEATURE_COMPAT_EXT_ATTR
+#define EXT2_FEATURE_INCOMPAT_SUPP (EXT4_FEATURE_INCOMPAT_FILETYPE| \
+ EXT4_FEATURE_INCOMPAT_META_BG)
+#define EXT2_FEATURE_RO_COMPAT_SUPP (EXT4_FEATURE_RO_COMPAT_SPARSE_SUPER| \
+ EXT4_FEATURE_RO_COMPAT_LARGE_FILE| \
+ EXT4_FEATURE_RO_COMPAT_BTREE_DIR)
+
+#define EXT3_FEATURE_COMPAT_SUPP EXT4_FEATURE_COMPAT_EXT_ATTR
+#define EXT3_FEATURE_INCOMPAT_SUPP (EXT4_FEATURE_INCOMPAT_FILETYPE| \
+ EXT4_FEATURE_INCOMPAT_RECOVER| \
+ EXT4_FEATURE_INCOMPAT_META_BG)
+#define EXT3_FEATURE_RO_COMPAT_SUPP (EXT4_FEATURE_RO_COMPAT_SPARSE_SUPER| \
+ EXT4_FEATURE_RO_COMPAT_LARGE_FILE| \
+ EXT4_FEATURE_RO_COMPAT_BTREE_DIR)
+
#define EXT4_FEATURE_COMPAT_SUPP EXT2_FEATURE_COMPAT_EXT_ATTR
#define EXT4_FEATURE_INCOMPAT_SUPP (EXT4_FEATURE_INCOMPAT_FILETYPE| \
EXT4_FEATURE_INCOMPAT_RECOVER| \
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 8553dfb..bb2d660 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -75,11 +75,27 @@ static void ext4_write_super(struct super_block *sb);
static int ext4_freeze(struct super_block *sb);
static struct dentry *ext4_mount(struct file_system_type *fs_type, int flags,
const char *dev_name, void *data);
+static inline int ext2_feature_set_ok(struct super_block *sb);
+static inline int ext3_feature_set_ok(struct super_block *sb);
static int ext4_feature_set_ok(struct super_block *sb, int readonly);
static void ext4_destroy_lazyinit_thread(void);
static void ext4_unregister_li_request(struct super_block *sb);
static void ext4_clear_request_list(void);
+#if !defined(CONFIG_EXT2_FS) && !defined(CONFIG_EXT2_FS_MODULE) && defined(CONFIG_EXT4_USE_FOR_EXT23)
+static struct file_system_type ext2_fs_type = {
+ .owner = THIS_MODULE,
+ .name = "ext2",
+ .mount = ext4_mount,
+ .kill_sb = kill_block_super,
+ .fs_flags = FS_REQUIRES_DEV,
+};
+#define IS_EXT2_SB(sb) ((sb)->s_bdev->bd_holder == &ext2_fs_type)
+#else
+#define IS_EXT2_SB(sb) (0)
+#endif
+
+
#if !defined(CONFIG_EXT3_FS) && !defined(CONFIG_EXT3_FS_MODULE) && defined(CONFIG_EXT4_USE_FOR_EXT23)
static struct file_system_type ext3_fs_type = {
.owner = THIS_MODULE,
@@ -3187,6 +3203,28 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
"feature flags set on rev 0 fs, "
"running e2fsck is recommended");
+ if (IS_EXT2_SB(sb)) {
+ if (ext2_feature_set_ok(sb))
+ ext4_msg(sb, KERN_INFO, "mounting ext2 file system "
+ "using the ext4 subsystem");
+ else {
+ ext4_msg(sb, KERN_ERR, "couldn't mount as ext2 due "
+ "to feature incompatibilities");
+ goto failed_mount;
+ }
+ }
+
+ if (IS_EXT3_SB(sb)) {
+ if (ext3_feature_set_ok(sb))
+ ext4_msg(sb, KERN_INFO, "mounting ext3 file system "
+ "using the ext4 subsystem");
+ else {
+ ext4_msg(sb, KERN_ERR, "couldn't mount as ext3 due "
+ "to feature incompatibilities");
+ goto failed_mount;
+ }
+ }
+
/*
* Check feature flags regardless of the revision level, since we
* previously didn't change the revision level when setting the flags,
@@ -4772,14 +4810,6 @@ static struct dentry *ext4_mount(struct file_system_type *fs_type, int flags,
}
#if !defined(CONFIG_EXT2_FS) && !defined(CONFIG_EXT2_FS_MODULE) && defined(CONFIG_EXT4_USE_FOR_EXT23)
-static struct file_system_type ext2_fs_type = {
- .owner = THIS_MODULE,
- .name = "ext2",
- .mount = ext4_mount,
- .kill_sb = kill_block_super,
- .fs_flags = FS_REQUIRES_DEV,
-};
-
static inline void register_as_ext2(void)
{
int err = register_filesystem(&ext2_fs_type);
@@ -4792,10 +4822,22 @@ static inline void unregister_as_ext2(void)
{
unregister_filesystem(&ext2_fs_type);
}
+
+static inline int ext2_feature_set_ok(struct super_block *sb)
+{
+ if (EXT4_HAS_INCOMPAT_FEATURE(sb, ~EXT2_FEATURE_INCOMPAT_SUPP))
+ return 0;
+ if (sb->s_flags & MS_RDONLY)
+ return 1;
+ if (EXT4_HAS_RO_COMPAT_FEATURE(sb, ~EXT2_FEATURE_RO_COMPAT_SUPP))
+ return 0;
+ return 1;
+}
MODULE_ALIAS("ext2");
#else
static inline void register_as_ext2(void) { }
static inline void unregister_as_ext2(void) { }
+static inline int ext2_feature_set_ok(struct super_block *sb) { return 0; }
#endif
#if !defined(CONFIG_EXT3_FS) && !defined(CONFIG_EXT3_FS_MODULE) && defined(CONFIG_EXT4_USE_FOR_EXT23)
@@ -4811,10 +4853,24 @@ static inline void unregister_as_ext3(void)
{
unregister_filesystem(&ext3_fs_type);
}
+
+static inline int ext3_feature_set_ok(struct super_block *sb)
+{
+ if (EXT4_HAS_INCOMPAT_FEATURE(sb, ~EXT3_FEATURE_INCOMPAT_SUPP))
+ return 0;
+ if (!EXT4_HAS_COMPAT_FEATURE(sb, EXT4_FEATURE_COMPAT_HAS_JOURNAL))
+ return 0;
+ if (sb->s_flags & MS_RDONLY)
+ return 1;
+ if (EXT4_HAS_RO_COMPAT_FEATURE(sb, ~EXT3_FEATURE_RO_COMPAT_SUPP))
+ return 0;
+ return 1;
+}
MODULE_ALIAS("ext3");
#else
static inline void register_as_ext3(void) { }
static inline void unregister_as_ext3(void) { }
+static inline int ext3_feature_set_ok(struct super_block *sb) { return 0; }
#endif
static struct file_system_type ext4_fs_type = {
--
1.7.3.1