Y2038 bug in ext4 recently_deleted() function

[Andreas Dilger <adilger@dilger.ca>]

[-- Attachment #1: Type: text/plain, Size: 1994 bytes --]

On Aug 17, 2017, at 3:21 AM, Jan Kara <jack@suse.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






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]