linux-ext4 - Re: Journal under-reservation bug on first >2G file

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: <542C1314.3030603@redhat.com>
Date:	Wed, 01 Oct 2014 09:43:32 -0500
From:	Eric Sandeen <sandeen@...hat.com>
To:	"Theodore Ts'o" <tytso@....edu>, Andreas Dilger <adilger@...ger.ca>
CC:	ext4 development <linux-ext4@...r.kernel.org>
Subject: Re: Journal under-reservation bug on first >2G file

On 10/1/14 6:53 AM, Theodore Ts'o wrote:
> On Tue, Sep 30, 2014 at 03:36:17PM -0600, Andreas Dilger wrote:
>>>
>>>  1.5a) Always set the large_file feature with a fresh mkfs, insteadl
>>>        of relying on the accident of the resize inode being > 2G!
>>
>> I think that 1.5a is definitely the way to go for new mke2fs, I'm a
>> bit surprised that we didn't do this for "-t ext4" a long time ago
>> given that we've enabled lots of other features automatically.
> 
> Yes, I agree that would be a good thing to do.  I'll make the change
> to mke2fs.conf.
> 
>> There shouldn't be any problem to do this retroactively in e2fsck
>> and potentially at mount time for filesystems that already have some
>> features enabled that are post-large_file (e.g. extents, flex_bg, etc.)
>> This definitely would not impose any compatibility issues, because any
>> kernel that supports those features already understands large_file.
> 
> That sounds like a plan.  If we only enable it automatically at mount
> time (iff we mounted the file system read/write) if any of the ext3 or
> ext4 specific features are enabled, that should be completely safe.

Ok, so do that, and don't bump the reservations? I suppose
the size test & superblock write can be removed, then...

This does bug me a little; at one point we were very carefully not
enabling any new features by mounting with a new kernel; that was
specific to mounting-ext2-with-ext4 etc, but it still feels slightly
inconsistent.  Although I guess we enable it today by mounting-and-
writing-a-big-enough-file.

Something like this should fix it too, though, with less unexpected
behind-your-back behavior:

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 3aa26e9..2f94cd6 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -2563,9 +2563,15 @@ retry_grab:
         * if there is delayed block allocation. But we still need
         * to journalling the i_disksize update if writes to the end
         * of file which has an already mapped buffer.
+        * If this write might need to update the superblock due to the
+        * filesize adding a new superblock feature flag, add that too.
         */
 retry_journal:
-       handle = ext4_journal_start(inode, EXT4_HT_WRITE_PAGE, 1);
+       handle = ext4_journal_start(inode, EXT4_HT_WRITE_PAGE,
+                                   EXT4_HAS_RO_COMPAT_FEATURE(inode->i_sb,
+                                       EXT4_FEATURE_RO_COMPAT_LARGE_FILE) ?
+                                   1 : 2);
+
        if (IS_ERR(handle)) {
                page_cache_release(page);
                return PTR_ERR(handle);


-ERic
--
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