Re: [PATCH v7 1/2] e2fsck: Correct ext4 dates generated by old kernels.

[Theodore Ts'o <tytso@mit.edu>]

On Sat, Dec 07, 2013 at 03:02:40PM -0500, David Turner wrote:
> 
> 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.

We can and should change time_to_string to take an unsigned 64-bit
type; it's an internal interface to debugfs.  The question of what to
do if the system time_t is only 32-bit.

What I'd probably suggest is to have a mode (set either by an
environment variable, or a debugfs command) which causes
time_to_string to use an internal version of a 64-bit capable gmtime
function.  This will allow for better regression testing, and it in a
way that will be have stable results regardless of whether a
particular platform, or a future version of glibc has a 64-bit time_t
or not.

> 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'm not sure we care, since nothing is actually using the dtime.  This
was originally intended to make it easy to recover from accident file
deletions, using debugfs's lsdel command.  However, ever since ext3,
in order to make sure the file system is consistent if it crashes
during an unlink, we end up truncating the file before we unlink it,
so i_blocks[] is completely cleared for deleted inodes.  This makes it
essentially impossible for lsdel to work.

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

Given that we always set ctime when delete a file (which makes sense,
since we are decrementing i_links_count), I think changing debugfs
(which is the only thing that even looks at dtime, BTW) makes a lot of
sense.

Regards,

						- Ted