Hello, all
I found there$B!G(Bs a contradiction between contents of some kernel warning
messages and the succeeding results, when an unsupported revision level
for EXT3 file system was detected in mounting process. In this case, the
messages said $B!H(BEXT3-fs warning: revision level too high, forcing
read-only mode$B!I(B. However, the warned file system was mounted with
ordinary read and write enabled mode actually, rather than read only
mode. The operation sequence described below shows a way to reproduce
this problem easily.
I think the messages should be changed, if the resultant mount mode is
valid, or the related mount code should be changed so that the file
system would be mounted with read only mode according to the warning
message. What do you think?
Here is my quick observation of the related kernel code. The kernel
function ext3_setup_super() checks the revision level of the file system
to be mounted and return a status indicating read only mode (MS_RDONLY),
if the level is too high. However, the current ext3_fill_super(), which
calls ext3_setup_super(), is careless of this status. ext3_remount()
also seem to fail to handle the returned status appropriately. In
addition, the corresponding code for EXT2 file system has the same problem.
--------------------
# mkfs -t ext3 -r 2 /dev/sdb1
mke2fs 1.39 (29-May-2006)
Filesystem label=
OS type: Linux
Block size=4096 (log=2)
Fragment size=4096 (log=2)
8962048 inodes, 17920499 blocks
896024 blocks (5.00%) reserved for the super user
First data block=0
Maximum filesystem blocks=0
547 block groups
32768 blocks per group, 32768 fragments per group
16384 inodes per group
Superblock backups stored on blocks:
32768, 98304, 163840, 229376, 294912, 819200, 884736, 1605632, 2654208,
4096000, 7962624, 11239424
Writing inode tables: done
Creating journal (32768 blocks): done
Writing superblocks and filesystem accounting information: done
This filesystem will be automatically checked every 34 mounts or
180 days, whichever comes first. Use tune2fs -c or -i to override.
# mount -t ext3 /dev/sdb1 /mnt/sdb1
# mount
/dev/mapper/VolGroup00-LogVol00 on / type ext3 (rw)
proc on /proc type proc (rw)
sysfs on /sys type sysfs (rw)
devpts on /dev/pts type devpts (rw,gid=5,mode=620)
/dev/sda1 on /boot type ext3 (rw)
tmpfs on /dev/shm type tmpfs (rw)
none on /proc/sys/fs/binfmt_misc type binfmt_misc (rw)
sunrpc on /var/lib/nfs/rpc_pipefs type rpc_pipefs (rw)
/dev/sdb1 on /mnt/sdb1 type ext3 (rw)
# cd /mnt/sdb1
# touch aaa.txt
# ls
aaa.txt lost+found
>From /var/log/messages:
Jul 17 15:45:54 kernel kernel: EXT3-fs warning: revision level too high,
forcing read-only mode
Jul 17 15:45:54 kernel kernel: EXT3 FS on sdb1, internal journal
Jul 17 15:45:54 kernel kernel: EXT3-fs: mounted filesystem with ordered
data mode.
--------------------
Thanks,
Tadao Uchiyama
>
> Hello, all
>
> I found there$B!G(Bs a contradiction between contents of some kernel warning messages and the succeeding results, when an unsupported revision level for EXT3 file system was detected in mounting process. In this case, the messages said $B!H(BEXT3-fs warning: revision level too high, forcing read-only mode$B!I(B. However, the warned file system was mounted with ordinary read and write enabled mode actually, rather than read only mode. The operation sequence described below shows a way to reproduce this problem easily.
>
> I think the messages should be changed, if the resultant mount mode is valid, or the related mount code should be changed so that the file
> system would be mounted with read only mode according to the warning message. What do you think?
>
> Here is my quick observation of the related kernel code. The kernel function ext3_setup_super() checks the revision level of the file system to be mounted and return a status indicating read only mode (MS_RDONLY), if the level is too high. However, the current ext3_fill_super(), which calls ext3_setup_super(), is careless of this status. ext3_remount() also seem to fail to handle the returned status appropriately. In addition, the corresponding code for EXT2 file system has the same problem.
>
>
Hi,
Assuming that the intention of the warning message ($B!H(BEXT3-fs warning:
revision level too high, forcing read only mode$B!I(B) is valid, I made the
following patch based on linux-2.6.26.2. This will force an EXT3 file
system with unsupported revision to be mounted with read only mode for
both mount case and remount case, even though read and write enabled
mode is specified or assumed as default for mounting. Any comments would
be highly appreciated. The test results with this patch is attached below.
----------
diff -up linux-2.6.26.2/fs/ext3/super.c.orig linux-2.6.26.2/fs/ext3/super.c
--- linux-2.6.26.2/fs/ext3/super.c.orig 2008-08-18 11:01:02.000000000 +0900
+++ linux-2.6.26.2/fs/ext3/super.c 2008-08-18 11:06:29.000000000 +0900
@@ -1898,7 +1898,8 @@ static int ext3_fill_super (struct super
goto failed_mount4;
}
- ext3_setup_super (sb, es, sb->s_flags & MS_RDONLY);
+ if (ext3_setup_super (sb, es, sb->s_flags & MS_RDONLY))
+ sb->s_flags |= MS_RDONLY;
/*
* akpm: core read_super() calls in here with the superblock locked.
* That deadlocks, because orphan cleanup needs to lock the superblock
@@ -2506,8 +2507,8 @@ static int ext3_remount (struct super_bl
sbi->s_mount_state = le16_to_cpu(es->s_state);
if ((err = ext3_group_extend(sb, es, n_blocks_count)))
goto restore_opts;
- if (!ext3_setup_super (sb, es, 0))
- sb->s_flags &= ~MS_RDONLY;
+ if (ext3_setup_super (sb, es, 0))
+ *flags &= ~MS_RDONLY;
}
}
#ifdef CONFIG_QUOTA
----------
There$B!G(Bs one thing I have to add. Even if the patch works and read only
mode is forced for an EXT3 file system with unsupported revision, the
mount mode for the file system shown by mount command or /etc/mtab can
be still $B!H(Brw$B!I(B, rather than $B!H(Bro$B!I(B. I think this issue should be cared not
only by kernel, but also by mount command itself. Any suggestion would
be highly appreciated for this issue also.
Thanks a lot.
Signed-off-by :Tadao Uchiyama <[email protected]>
The test results with the patch as above mentioned:
----------
# mkfs -t ext3 -r 2 /dev/sdb1
mke2fs 1.39 (29-May-2006)
Filesystem label=
OS type: Linux
Block size=4096 (log=2)
Fragment size=4096 (log=2)
3014656 inodes, 6024367 blocks
301218 blocks (5.00%) reserved for the super user
First data block=0
Maximum filesystem blocks=0
184 block groups
32768 blocks per group, 32768 fragments per group
16384 inodes per group
Superblock backups stored on blocks:
32768, 98304, 163840, 229376, 294912, 819200, 884736, 1605632, 2654208,
4096000
Writing inode tables: done
Creating journal (32768 blocks): done
Writing superblocks and filesystem accounting information: done
This filesystem will be automatically checked every 34 mounts or
180 days, whichever comes first. Use tune2fs -c or -i to override.
# mount -t ext3 /dev/sdb1 /mnt/sdb1
# cd /mnt/sdb1
# touch aaa.txt
touch: cannot touch `aaa.txt': Read-only file system
# mount
/dev/mapper/VolGroup00-LogVol00 on / type ext3 (rw)
proc on /proc type proc (rw)
sysfs on /sys type sysfs (rw)
devpts on /dev/pts type devpts (rw,gid=5,mode=620)
/dev/sda1 on /boot type ext3 (rw)
tmpfs on /dev/shm type tmpfs (rw)
/dev/sdb1 on /mnt/sdb1 type ext3 (rw)
# cd /tmp
# mount -o rw,remount /mnt/sdb1
# cd /mnt/sdb1
# touch aaa.txt
touch: cannot touch `aaa.txt': Read-only file system
# mount
/dev/mapper/VolGroup00-LogVol00 on / type ext3 (rw)
proc on /proc type proc (rw)
sysfs on /sys type sysfs (rw)
devpts on /dev/pts type devpts (rw,gid=5,mode=620)
/dev/sda1 on /boot type ext3 (rw)
tmpfs on /dev/shm type tmpfs (rw)
/dev/sdb1 on /mnt/sdb1 type ext3 (rw)
----------
> ----------
> diff -up linux-2.6.26.2/fs/ext3/super.c.orig linux-2.6.26.2/fs/ext3/super.c
> --- linux-2.6.26.2/fs/ext3/super.c.orig 2008-08-18 11:01:02.000000000 +0900
> +++ linux-2.6.26.2/fs/ext3/super.c 2008-08-18 11:06:29.000000000 +0900
> @@ -1898,7 +1898,8 @@ static int ext3_fill_super (struct super
> goto failed_mount4;
> }
>
> - ext3_setup_super (sb, es, sb->s_flags & MS_RDONLY);
> + if (ext3_setup_super (sb, es, sb->s_flags & MS_RDONLY))
> + sb->s_flags |= MS_RDONLY;
> /*
> * akpm: core read_super() calls in here with the superblock locked.
> * That deadlocks, because orphan cleanup needs to lock the superblock
> @@ -2506,8 +2507,8 @@ static int ext3_remount (struct super_bl
> sbi->s_mount_state = le16_to_cpu(es->s_state);
> if ((err = ext3_group_extend(sb, es, n_blocks_count)))
> goto restore_opts;
> - if (!ext3_setup_super (sb, es, 0))
> - sb->s_flags &= ~MS_RDONLY;
> + if (ext3_setup_super (sb, es, 0))
> + *flags &= ~MS_RDONLY;
> }
> }
> #ifdef CONFIG_QUOTA
> ----------
>
Since the patch described in my previous response was formatted inappropriately with no indents, let
me please send it here again.
Thanks.
Signed-off-by :Tadao Uchiyama <[email protected]>
----------
diff -up linux-2.6.26.2/fs/ext3/super.c.orig linux-2.6.26.2/fs/ext3/super.c
--- linux-2.6.26.2/fs/ext3/super.c.orig 2008-08-18 11:01:02.000000000 +0900
+++ linux-2.6.26.2/fs/ext3/super.c 2008-08-18 11:06:29.000000000 +0900
@@ -1898,7 +1898,8 @@ static int ext3_fill_super (struct super
goto failed_mount4;
}
- ext3_setup_super (sb, es, sb->s_flags & MS_RDONLY);
+ if (ext3_setup_super (sb, es, sb->s_flags & MS_RDONLY))
+ sb->s_flags |= MS_RDONLY;
/*
* akpm: core read_super() calls in here with the superblock locked.
* That deadlocks, because orphan cleanup needs to lock the superblock
@@ -2506,8 +2507,8 @@ static int ext3_remount (struct super_bl
sbi->s_mount_state = le16_to_cpu(es->s_state);
if ((err = ext3_group_extend(sb, es, n_blocks_count)))
goto restore_opts;
- if (!ext3_setup_super (sb, es, 0))
- sb->s_flags &= ~MS_RDONLY;
+ if (ext3_setup_super (sb, es, 0))
+ *flags &= ~MS_RDONLY;
}
}
#ifdef CONFIG_QUOTA
----------
> Since the patch described in my previous response was formatted inappropriately with no indents, let
> me please send it here again.
>
> Thanks.
> Signed-off-by :Tadao Uchiyama <[email protected]>
>
> ----------
> diff -up linux-2.6.26.2/fs/ext3/super.c.orig linux-2.6.26.2/fs/ext3/super.c
For what it's worth, ext2 and ext4 have the same problems...
> --- linux-2.6.26.2/fs/ext3/super.c.orig 2008-08-18 11:01:02.000000000 +0900
> +++ linux-2.6.26.2/fs/ext3/super.c 2008-08-18 11:06:29.000000000 +0900
> @@ -1898,7 +1898,8 @@ static int ext3_fill_super (struct super
> goto failed_mount4;
> }
>
> - ext3_setup_super (sb, es, sb->s_flags & MS_RDONLY);
> + if (ext3_setup_super (sb, es, sb->s_flags & MS_RDONLY))
> + sb->s_flags |= MS_RDONLY;
> /*
> * akpm: core read_super() calls in here with the superblock locked.
> * That deadlocks, because orphan cleanup needs to lock the superblock
> @@ -2506,8 +2507,8 @@ static int ext3_remount (struct super_bl
> sbi->s_mount_state = le16_to_cpu(es->s_state);
> if ((err = ext3_group_extend(sb, es, n_blocks_count)))
One other thing I worry about; we are doing group_extend before we check
the revision. Is that safe? We also still do things like replay the
journal and process orphan inodes before checking the revision.
Should we even worry about this, or is the revision level never going to
change again, and we can almost just ignore it now? (or assume that for
recent kernels, a too-high rev level indicates corruption and fail the
mount?)
-Eric
> goto restore_opts;
> - if (!ext3_setup_super (sb, es, 0))
> - sb->s_flags &= ~MS_RDONLY;
> + if (ext3_setup_super (sb, es, 0))
> + *flags &= ~MS_RDONLY;
> }
> }
> #ifdef CONFIG_QUOTA
> ----------
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> For what it's worth, ext2 and ext4 have the same problems...
Thanks a lot for the first response! I really appreciate it.
>> --- linux-2.6.26.2/fs/ext3/super.c.orig 2008-08-18 11:01:02.000000000 +0900
>> +++ linux-2.6.26.2/fs/ext3/super.c 2008-08-18 11:06:29.000000000 +0900
>> @@ -1898,7 +1898,8 @@ static int ext3_fill_super (struct super
>> goto failed_mount4;
>> }
>>
>> - ext3_setup_super (sb, es, sb->s_flags & MS_RDONLY);
>> + if (ext3_setup_super (sb, es, sb->s_flags & MS_RDONLY))
>> + sb->s_flags |= MS_RDONLY;
>> /*
>> * akpm: core read_super() calls in here with the superblock locked.
>> * That deadlocks, because orphan cleanup needs to lock the
>> superblock @@ -2506,8 +2507,8 @@ static int ext3_remount (struct super_bl
>> sbi->s_mount_state = le16_to_cpu(es->s_state);
>> if ((err = ext3_group_extend(sb, es,
>> n_blocks_count)))
> One other thing I worry about; we are doing group_extend before we check
> the revision. Is that safe? We also still do things like replay the
> journal and process orphan inodes before checking the revision.
>
> Should we even worry about this, or is the revision level never going
> to change again, and we can almost just ignore it > > now? (or assume
> that for recent kernels, a too-high rev level indicates corruption and
> fail the
> mount?)
>
> -Eric
I think I need more time to find an appropriate answer, but I'll be keeping busy for a while. Please wait. Could anyone give us some suggestion? Any comments would be highly appreciated.
> > goto restore_opts;
> > - if (!ext3_setup_super (sb, es, 0))
> > - sb->s_flags &= ~MS_RDONLY;
> > + if (ext3_setup_super (sb, es, 0))
> > + *flags &= ~MS_RDONLY;
> > }
> > }
> > #ifdef CONFIG_QUOTA
> > ----------
> > --
Thanks.
Signed-off-by :Tadao Uchiyama <[email protected]>
>> ----------
>> diff -up linux-2.6.26.2/fs/ext3/super.c.orig
>> linux-2.6.26.2/fs/ext3/super.c
>
> For what it's worth, ext2 and ext4 have the same problems...
>
>> --- linux-2.6.26.2/fs/ext3/super.c.orig 2008-08-18 11:01:02.000000000
>> +0900
>> +++ linux-2.6.26.2/fs/ext3/super.c 2008-08-18 11:06:29.000000000 +0900
>> @@ -1898,7 +1898,8 @@ static int ext3_fill_super (struct super
>> goto failed_mount4;
>> }
>>
>> - ext3_setup_super (sb, es, sb->s_flags & MS_RDONLY);
>> + if (ext3_setup_super (sb, es, sb->s_flags & MS_RDONLY))
>> + sb->s_flags |= MS_RDONLY;
>> /*
>> * akpm: core read_super() calls in here with the superblock locked.
>> * That deadlocks, because orphan cleanup needs to lock the
>> superblock @@ -2506,8 +2507,8 @@ static int ext3_remount (struct super_bl
>> sbi->s_mount_state = le16_to_cpu(es->s_state);
>> if ((err = ext3_group_extend(sb, es,
>> n_blocks_count)))
>
> One other thing I worry about; we are doing group_extend before we check the revision. Is that safe? We also still do things like replay the journal and process orphan inodes before checking the revision.
>
> Should we even worry about this, or is the revision level never going to change again, and we can almost just ignore it now? (or assume that for recent kernels, a too-high rev level indicates corruption and fail the
> mount?)
>
> -Eric
>
Sorry for the long delay in my response.
I agree that the mount of a file with a too-high revision level should be rejected,
if the current revision level is never going to change again, because the too-high
revision level must be an indication of some corruption in this case.
The problem is when we should fail the mount. It seems to be too late to fail the mount
after the related super block has been updated in group_extend or clear_journal_error.
It?ll be safe to make the revision somewhere earlier stage, at least before doing
clear_journal_error and group_extend.
If the current revision level can be changed in the near feature, the recommended action
will depend on how a file system with the next revision is implemented. If it must be
mounted with read only mode as the current revision check code in setup_super intends,
the revision check also should be made before doing any process which may involve
updating the related super block.
Anyway, the original problem I like to let everyone know is that the current ext2/ext3/ext4
code allows a file with a too-high revision level to be mounted with read & write enabled,
while the following error message is issued by the revision check:
"EXTx-fs warning: revision level too high, forcing read-only mode"
Please remember the problem again and let me know your opinion what we should do for it.
The code should be updated so that the file would be mounted with read only mode or
the message should be changed?
Thank you.
Signed-off-by :Tadao Uchiyama <[email protected]>
>> goto restore_opts;
>> - if (!ext3_setup_super (sb, es, 0))
>> - sb->s_flags &= ~MS_RDONLY;
>> + if (ext3_setup_super (sb, es, 0))
>> + *flags &= ~MS_RDONLY;
>> }
>> }
>> #ifdef CONFIG_QUOTA
>> ----------
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-ext4"
>> in the body of a message to [email protected] More majordomo
>> info at http://vger.kernel.org/majordomo-info.html
> .
>
On Tue, Feb 17, 2009 at 05:50:44PM +0900, Tadao Uchiyama wrote:
> Sorry for the long delay in my response. I agree that the mount of
> a file with a too-high revision level should be rejected, if the
> current revision level is never going to change again, because the
> too-high revision level must be an indication of some corruption in
> this case. The problem is when we should fail the mount. It seems
> to be too late to fail the mount after the related super block has
> been updated in group_extend or clear_journal_error. It’ll be safe
> to make the revision somewhere earlier stage, at least before doing
> clear_journal_error and group_extend.
I'd just probably add something right after the magic number check
(i.e., around line 2072 in fs/ext4/super.c, in ext4_fill_super()).
It's highly unlikely we would ever change the revision number at this
point, given that we have the feature compatibility bitmasks as the
primary way we indicate format changes in the filesystem these days.
So I wouldn't even allow a read-only mount, I'd just fail the mount
altogether, and very early; basically, treat it as part of the magic
number.
- Ted