linux-ext4 - Re: EXT3 file system with unsupported revision level can be mounted in R/W mode

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-next>] [day] [month] [year] [list]

Message-ID: <499A7A64.8040608@uniadex.co.jp>
Date:	Tue, 17 Feb 2009 17:50:44 +0900
From:	Tadao Uchiyama <Tadao.Uchiyama@...adex.co.jp>
To:	sandeen@...hat.com
CC:	linux-ext4@...r.kernel.org
Subject: Re: EXT3 file system with unsupported revision level can be mounted
 in R/W mode


>> ----------
>> 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 <Tadao.Uchiyama@...adex.co.jp>

>>                                 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 majordomo@...r.kernel.org More majordomo
>> info at  http://vger.kernel.org/majordomo-info.html
> .
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html