From patchwork Sat Dec 7 20:02:40 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Turner X-Patchwork-Id: 298716 Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 444A52C00CE for ; Sun, 8 Dec 2013 07:03:15 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755661Ab3LGUCo (ORCPT ); Sat, 7 Dec 2013 15:02:44 -0500 Received: from caiajhbdccah.dreamhost.com ([208.97.132.207]:52784 "EHLO homiemail-a100.g.dreamhost.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1755576Ab3LGUCn (ORCPT ); Sat, 7 Dec 2013 15:02:43 -0500 Received: from homiemail-a100.g.dreamhost.com (localhost [127.0.0.1]) by homiemail-a100.g.dreamhost.com (Postfix) with ESMTP id 2515131A073; Sat, 7 Dec 2013 12:02:42 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=novalis.org; h=message-id :subject:from:to:cc:date:in-reply-to:references:content-type :mime-version:content-transfer-encoding; s=novalis.org; bh=vbDFG 9NUrIg2Ma+3zyGDzDlFuWI=; b=TJJNtueVgJxwqQhlzKJZ/TF4OMUH+V9pyfIKw CUXNn91B52AeWI6Wf7u5FK3e939jDvqTbVQW56uh2Zt1mGBgVeKzA7Y3rAEqC7Eu rf2PKPOnVK49x8gRSfAqdE453DOT3nEvixicOtMiwOKdFQKMgRIHtmKHkYc1BjPo 9zAliM= Received: from [10.0.1.4] (ool-4579628d.dyn.optonline.net [69.121.98.141]) (using SSLv3 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) (Authenticated sender: novalis@novalis.org) by homiemail-a100.g.dreamhost.com (Postfix) with ESMTPSA id 52E8331A070; Sat, 7 Dec 2013 12:02:41 -0800 (PST) Message-ID: <1386446560.10748.5.camel@chiang> Subject: [PATCH v7 1/2] e2fsck: Correct ext4 dates generated by old kernels. From: David Turner To: Andreas Dilger Cc: Mark Harris , Theodore Ts'o , Jan Kara , Ext4 Developers List , Linux Kernel Mailing List Date: Sat, 07 Dec 2013 15:02:40 -0500 In-Reply-To: References: <1383808590.23882.13.camel@chiang> <20131107160341.GA3850@quack.suse.cz> <1383864864.23882.33.camel@chiang> <20131107231445.GG2054@quack.suse.cz> <1383866807.23882.41.camel@chiang> <1383981551.8994.27.camel@chiang> <1384070214.8994.47.camel@chiang> <20131112003018.GA30281@thunk.org> <6DE0AF86-98E6-4DE9-BB7F-40FB32E1BC26@dilger.ca> <1384326020.8994.186.camel@chiang> <276FA06E-1EE0-4FB4-94E1-B6D9F05F0B5B@dilger.ca> <1384418641.1957.1.camel@chiang> <1384463193.1957.27.camel@chiang> <1385762083.20396.48.camel@chiang> X-Mailer: Evolution 3.6.4-0ubuntu1 Mime-Version: 1.0 Sender: linux-ext4-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-ext4@vger.kernel.org I went ahead and wrote some tests, and they seem to confirm that my patch to e2fsck works as expected (once I added crtime). However, as Andreas notes, "we want to verify .. that "debugfs -R 'stat testfile'" decodes the times correctly." Unfortunately, it does not, and it is not trivial to fix. debugfs uses an internal function time_to_string(__u32), which calls gmtime or localtime. These functions are defined on time_t, which is 32-bits on 32-bit Linux systems. In addition, because time_to_string takes an unsigned int, it returns bad results for pre-1970 dates. One fix for this would be to include our own gmtime/localtime defined on 64-bit integers. But this is likely to be complicated, given the dependence of localtime on timezone data. Instead, I decided to simply adjust time_to_string to take a time_t. This means that on 32-bit time_t systems, we will give wrong results for post-2038 dates, but correct results for pre-1970 dates. (see patch 2/2) For the tests, I am looking at the raw value of ctime and ctime_extra, which will work regardless of the size of time_t Is using time_t acceptable, or do you want me to copy over gmtime/localtime from glibc? Or is there a third approach that would be better? Also, while I was making this change, I happened to notice that there is no dtime_extra field into which to put the extra bits. This means that there is still a year 2038 problem with tools that deal with deleted files (including the corner case that the system is shut down cleanly at the exact second that the bottom 32 bits of the current time are zero, leading fsck to believe that there was an unclean shutdown). I want to make an additional change to correct this. Since I assume it is impossible to add an additional field, I propose to use ctime_extra to store the extra bits of dtime on deletion. This is a bit hackish, since it does destroy a bit of data, but it is significantly better than dtime being totally broken after 2038. What do people think? --- Older kernels on 64-bit machines would incorrectly encode pre-1970 ext4 dates as post-2311 dates. Detect and correct this (assuming the current date is before 2242). Includes tests for this, as well as changes to debugfs to correctly set crtimes. Signed-off-by: David Turner --- debugfs/set_fields.c | 2 +- e2fsck/pass1.c | 43 ++++++++++++++++ e2fsck/problem.c | 4 ++ e2fsck/problem.h | 4 ++ lib/extra_epoch.h | 2 + tests/f_pre_1970_date_encoding/expect | 45 ++++++++++++++++ tests/f_pre_1970_date_encoding/name | 1 + tests/f_pre_1970_date_encoding/script | 96 +++++++++++++++++++++++++++++++++++ 8 files changed, 196 insertions(+), 1 deletion(-) create mode 100644 lib/extra_epoch.h create mode 100644 tests/f_pre_1970_date_encoding/expect create mode 100644 tests/f_pre_1970_date_encoding/name create mode 100644 tests/f_pre_1970_date_encoding/script diff --git a/debugfs/set_fields.c b/debugfs/set_fields.c index aad1cd8..f7c55a7 100644 --- a/debugfs/set_fields.c +++ b/debugfs/set_fields.c @@ -200,7 +200,7 @@ static struct field_set_info inode_fields[] = { 4, parse_uint }, { "atime_extra", &set_inode.i_atime_extra, NULL, 4, parse_uint }, - { "crtime", &set_inode.i_crtime, NULL, 4, parse_uint }, + { "crtime", &set_inode.i_crtime, NULL, 4, parse_time }, { "crtime_extra", &set_inode.i_crtime_extra, NULL, 4, parse_uint }, { "bmap", NULL, NULL, 4, parse_bmap, FLAG_ARRAY }, diff --git a/e2fsck/pass1.c b/e2fsck/pass1.c index ab23e42..ecbd79e 100644 --- a/e2fsck/pass1.c +++ b/e2fsck/pass1.c @@ -50,6 +50,8 @@ #include "problem.h" +#include "extra_epoch.h" + #ifdef NO_INLINE_FUNCS #define _INLINE_ #else @@ -348,6 +350,21 @@ fix: EXT2_INODE_SIZE(sb), "pass1"); } +static int check_inode_extra_negative_epoch(__u32 xtime, __u32 extra) { + return (xtime & (1 << 31)) != 0 && + (extra & EXT4_EPOCH_MASK) == EXT4_EPOCH_MASK; +} + +#define CHECK_INODE_EXTRA_NEGATIVE_EPOCH(inode, xtime) \ + check_inode_extra_negative_epoch(inode->i_##xtime, \ + inode->i_##xtime##_extra) + +/* When today's date is earlier than 2242, we assume that atimes, + * ctimes, crtimes, and mtimes with years in the range 2310..2378 are + * actually pre-1970 dates mis-encoded. + */ +#define EXT4_EXTRA_NEGATIVE_DATE_CUTOFF 2 * (1LL << 32) + static void check_inode_extra_space(e2fsck_t ctx, struct problem_context *pctx) { struct ext2_super_block *sb = ctx->fs->super; @@ -388,6 +405,32 @@ static void check_inode_extra_space(e2fsck_t ctx, struct problem_context *pctx) /* it seems inode has an extended attribute(s) in body */ check_ea_in_inode(ctx, pctx); } + + /* + * If the inode's extended atime (ctime, crtime, mtime) is stored in + * the old, invalid format, repair it. + */ + if (sizeof(time_t) > 4 && ctx->now < EXT4_EXTRA_NEGATIVE_DATE_CUTOFF && + (CHECK_INODE_EXTRA_NEGATIVE_EPOCH(inode, atime) || + CHECK_INODE_EXTRA_NEGATIVE_EPOCH(inode, ctime) || + CHECK_INODE_EXTRA_NEGATIVE_EPOCH(inode, crtime) || + CHECK_INODE_EXTRA_NEGATIVE_EPOCH(inode, mtime))) { + + if (!fix_problem(ctx, PR_1_EA_TIME_OUT_OF_RANGE, pctx)) + return; + + if (CHECK_INODE_EXTRA_NEGATIVE_EPOCH(inode, atime)) + inode->i_atime_extra &= ~EXT4_EPOCH_MASK; + if (CHECK_INODE_EXTRA_NEGATIVE_EPOCH(inode, ctime)) + inode->i_ctime_extra &= ~EXT4_EPOCH_MASK; + if (CHECK_INODE_EXTRA_NEGATIVE_EPOCH(inode, crtime)) + inode->i_crtime_extra &= ~EXT4_EPOCH_MASK; + if (CHECK_INODE_EXTRA_NEGATIVE_EPOCH(inode, mtime)) + inode->i_mtime_extra &= ~EXT4_EPOCH_MASK; + e2fsck_write_inode_full(ctx, pctx->ino, pctx->inode, + EXT2_INODE_SIZE(sb), "pass1"); + } + } /* diff --git a/e2fsck/problem.c b/e2fsck/problem.c index 897693a..b212d00 100644 --- a/e2fsck/problem.c +++ b/e2fsck/problem.c @@ -1018,6 +1018,10 @@ static struct e2fsck_problem problem_table[] = { N_("@i %i, end of extent exceeds allowed value\n\t(logical @b %c, physical @b %b, len %N)\n"), PROMPT_CLEAR, 0 }, + /* Timestamp(s) on inode beyond 2310-04-04 are likely pre-1970 dates. */ + { PR_1_EA_TIME_OUT_OF_RANGE, + N_("Timestamp(s) on @i %i beyond 2310-04-04 are likely pre-1970 dates.\n"), + PROMPT_FIX | PR_PREEN_OK | PR_NO_OK, 0 }, /* Pass 1b errors */ diff --git a/e2fsck/problem.h b/e2fsck/problem.h index ae1ed26..3710638 100644 --- a/e2fsck/problem.h +++ b/e2fsck/problem.h @@ -593,6 +593,10 @@ struct problem_context { #define PR_1_EXTENT_INDEX_START_INVALID 0x01006D #define PR_1_EXTENT_END_OUT_OF_BOUNDS 0x01006E + +/* Timestamp(s) on inode beyond 2310-04-04 are likely pre-1970 dates. */ +#define PR_1_EA_TIME_OUT_OF_RANGE 0x01006F + /* * Pass 1b errors */ diff --git a/lib/extra_epoch.h b/lib/extra_epoch.h new file mode 100644 index 0000000..465c43f --- /dev/null +++ b/lib/extra_epoch.h @@ -0,0 +1,2 @@ +#define EXT4_EPOCH_BITS 2 +#define EXT4_EPOCH_MASK ((1 << EXT4_EPOCH_BITS) - 1) diff --git a/tests/f_pre_1970_date_encoding/expect b/tests/f_pre_1970_date_encoding/expect new file mode 100644 index 0000000..1a71571 --- /dev/null +++ b/tests/f_pre_1970_date_encoding/expect @@ -0,0 +1,45 @@ +times for year-1909 = + ctime: 0x8e475440:00000003 + atime: 0x8e475440:00000003 + mtime: 0x8e475440:00000003 +crtime: 0x8e475440:00000003 +times for year-1979 = + ctime: 0x11db6940:00000000 + atime: 0x11db6940:00000000 + mtime: 0x11db6940:00000000 +crtime: 0x11db6940:00000000 +times for year-2039 = + ctime: 0x82a37b40:00000001 + atime: 0x82a37b40:00000001 + mtime: 0x82a37b40:00000001 +crtime: 0x82a37b40:00000001 +times for year-2139 = + ctime: 0x3e9b9940:00000001 + atime: 0x3e9b9940:00000001 + mtime: 0x3e9b9940:00000001 +crtime: 0x3e9b9940:00000001 +times for year-1909 = + ctime: 0x8e475440:00000000 + atime: 0x8e475440:00000000 + mtime: 0x8e475440:00000000 +crtime: 0x8e475440:00000000 +times for year-1979 = + ctime: 0x11db6940:00000000 + atime: 0x11db6940:00000000 + mtime: 0x11db6940:00000000 +crtime: 0x11db6940:00000000 +times for year-2039 = + ctime: 0x82a37b40:00000001 + atime: 0x82a37b40:00000001 + mtime: 0x82a37b40:00000001 +crtime: 0x82a37b40:00000001 +times for year-2139 = + ctime: 0x3e9b9940:00000001 + atime: 0x3e9b9940:00000001 + mtime: 0x3e9b9940:00000001 +crtime: 0x3e9b9940:00000001 +times for year-1909 = + ctime: 0x8e475440:00000003 + atime: 0x8e475440:00000003 + mtime: 0x8e475440:00000003 +crtime: 0x8e475440:00000003 diff --git a/tests/f_pre_1970_date_encoding/name b/tests/f_pre_1970_date_encoding/name new file mode 100644 index 0000000..9805324 --- /dev/null +++ b/tests/f_pre_1970_date_encoding/name @@ -0,0 +1 @@ +correct mis-encoded pre-1970 dates diff --git a/tests/f_pre_1970_date_encoding/script b/tests/f_pre_1970_date_encoding/script new file mode 100644 index 0000000..c3e12f5 --- /dev/null +++ b/tests/f_pre_1970_date_encoding/script @@ -0,0 +1,96 @@ +if test -x $DEBUGFS_EXE; then + +OUT=$test_name.log +TIMESTAMPS=$test_name.timestamps.log +EXP=$test_dir/expect +FSCK_OPT=-yf + +create_file_with_xtime_and_extra() { + name=$1 + time=$2 + extra=$3 + $DEBUGFS -w -R "write /dev/null $name" $TMPFILE > $OUT 2>&1 + for xtime in atime ctime mtime crtime + do + $DEBUGFS -w -R "set_inode_field $name $xtime @$time" $TMPFILE > $OUT 2>&1 + + $DEBUGFS -w -R "set_inode_field $name ${xtime}_extra $extra" $TMPFILE > $OUT 2>&1 + done +} + +get_file_xtime_and_extra() { + name=$1 + echo "times for $name =" >> $TIMESTAMPS + $DEBUGFS -R "stat $name" $TMPFILE 2>&1 | egrep '^( a| c| m|cr)time: ' |sed 's/ --.*//' >> $TIMESTAMPS +} + +rm -f $OUT +rm -f $TIMESTAMPS + +#create an empty ext4 filesystem with 256-byte inodes for testing +dd status=none if=/dev/zero of=$TMPFILE bs=1024 count=5000 +echo mkfs.ext4 -q -I 256 $TMPFILE >> $OUT +yes | mkfs.ext4 -q -I 256 $TMPFILE >> $OUT 2>&1 + +# this is a pre-1970 file encoded with the old encoding. +# fsck should repair this +create_file_with_xtime_and_extra year-1909 -1907928000 3 + +# these are all already encoded correctly +create_file_with_xtime_and_extra year-1979 299592000 0 +create_file_with_xtime_and_extra year-2039 2191752000 1 +create_file_with_xtime_and_extra year-2139 5345352000 1 + +# confirm that the xtime is wrong on the pre-1970 file +get_file_xtime_and_extra year-1909 + +# and confirm that it is right on the remaining files +get_file_xtime_and_extra year-1979 +get_file_xtime_and_extra year-2039 +get_file_xtime_and_extra year-2139 + +# before we repair the filesystem, save off a copy so that +# we can use it later + +cp $TMPFILE $TMPFILE.backup + +# repair the filesystem +E2FSCK_TIME=1386393539 $FSCK $FSCK_OPT $TMPFILE >> $OUT 2>&1 + +# check that the dates and xtime_extra on the file is now correct +get_file_xtime_and_extra year-1909 + +# check that the remaining dates have not been altered +get_file_xtime_and_extra year-1979 +get_file_xtime_and_extra year-2039 +get_file_xtime_and_extra year-2139 + +# now we need to check that after the year 2242, e2fsck does not +# modify dates with extra_xtime=3 + +# restore the unrepaired filesystem +mv $TMPFILE.backup $TMPFILE + +#retry the repair +E2FSCK_TIME=9270393539 $FSCK $FSCK_OPT $TMPFILE >> $OUT 2>&1 + +# check that the 1909 file is unaltered (i.e. it has a post-2378 date) +get_file_xtime_and_extra year-1909 + +cmp -s $TIMESTAMPS $EXP +status=$? + +if [ "$status" = 0 ] ; then + echo "$test_name: $test_description: ok" + touch $test_name.ok +else + echo "$test_name: $test_description: failed" + diff $DIFF_OPTS $EXP $TIMESTAMPS > $test_name.failed +fi + +unset OUT TIMESTAMPS EXP FSCK_OPT + +else #if test -x $DEBUGFS_EXE; then + echo "$test_name: $test_description: skipped" +fi +