linux-ext4 - Y2038 bug in ext4 recently

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: <BCA34CA8-FDDF-49FA-A5B4-9CCCEAC0C8D0@dilger.ca>
Date:   Thu, 17 Aug 2017 15:51:41 -0600
From:   Andreas Dilger <adilger@...ger.ca>
To:     Theodore Ts'o <tytso@....edu>,
        Deepa Dinamani <deepa.kernel@...il.com>,
        Arnd Bergmann <arnd@...db.de>
Cc:     Wang Shilong <wshilong@....com>,
        Wang Shilong <wangshilong1991@...il.com>,
        "linux-ext4@...r.kernel.org" <linux-ext4@...r.kernel.org>,
        Shuichi Ihara <sihara@....com>, Li Xi <lixi@....com>,
        Jan Kara <jack@...e.cz>
Subject: Y2038 bug in ext4 recently_deleted() function

On Aug 17, 2017, at 3:21 AM, Jan Kara <jack@...e.cz> wrote:
> 
> On Thu 17-08-17 11:19:59, Jan Kara wrote:
>> Hi Shilong!
>> 
>> On Thu 17-08-17 06:23:26, Wang Shilong wrote:
>>>     thanks for good suggestion, just one question we could not hold lock
>>> with nojounal mode, how about something attached one?
>>> 
>>> please let me know if you have better taste for it, much appreciated!
>> 
>> Thanks for quickly updating the patch! Is the only reason why you cannot
>> hold the lock in the nojournal mode that sb_getblk() might sleep? The
>> attached patch should fix that so that you don't have to special-case the
>> nojournal mode anymore.
> 
> Forgot to attach the patch - here it is. Feel free to include it in your
> series as a preparatory patch.

Strange, I never even knew recently_deleted() existed, even though it was
added to the tree 4 years ago yesterday.  It looks like this is only used
with the no-journal code, which I don't really interact with.

One thing I did notice when looking at it is that there is a Y2038 bug in
recently_deleted(), as it is comparing 32-bit i_dtime directly with 64-bit
get_seconds().  To fix this, it would be possible to either use a wrapped
32-bit comparison, like time_after() for jiffies, something like:

	u32 now, dtime;

	/* assume dtime is within the past 30 years, see time_after() */
        now = get_seconds();
	if (dtime && (dtime - now < 0) && (dtime + recentcy - now < 0))
		ret = 1;

or use i_ctime_extra to implicitly extend i_dtime beyond 2038, something like:

	/* assume dtime epoch same as ctime, see EXT4_INODE_GET_XTIME() */
	dtime = le32_to_cpu(raw_inode->i_dtime);
	if (EXT4_INODE_SIZE(sb) > EXT4_GOOD_OLD_INODE_SIZE &&
	    offsetof(typeof(*raw_inode), i_ctime_extra) + 4 <=
	    EXT4_GOOD_OLD_INODE_SIZE + le32_to_cpu(raw_inode->i_extra_isize))
                dtime += (long)(le32_to_cpu(raw_inode->i_ctime_extra) &
				EXT4_EPOCH_MASK) << 32;

Cheers, Andreas






Download attachment "signature.asc" of type "application/pgp-signature" (196 bytes)