All of lore.kernel.org
 help / color / mirror / Atom feed
* Files dated before 1970
@ 2020-06-20  2:16 Matthew Wilcox
  2020-06-20  8:59 ` Arnd Bergmann
  0 siblings, 1 reply; 5+ messages in thread
From: Matthew Wilcox @ 2020-06-20  2:16 UTC (permalink / raw)
  To: Deepa Dinamani; +Cc: linux-fsdevel, Arnd Bergmann


Hi Deepa,

Your commit 95582b008388 ("vfs: change inode times to use struct
timespec64") changed the behaviour of some filesystems with regards to
files from before 1970.  Specifically, this line from JFS, unchanged
since before 2.6.12:

fs/jfs/jfs_imap.c:3065: ip->i_atime.tv_sec = le32_to_cpu(dip->di_atime.tv_sec);

le32_to_cpu() returns a u32.  Before your patch, the u32 was assigned
to an s32, so a file with a date stamp of 1968 would show up that way.
After your patch, the u32 is zero-extended to an s64, so a file from
1968 now appears to be from 2104.

Obviously there aren't a lot of files around from before 1970, so it's
not surprising that nobody's noticed yet.  But I don't think this was
an intended change.

I see a similar problem in bfs, efs & qnx4.  Other filesystems might also
have the same problem, but I haven't done an intensive investigation.
This started as a bit of banter, and I inadvertently noticed this bug,
so I felt I had a moral imperative to report it.

Thanks for all the work you've done on the y2038 problem; it's really
important.  It might even be the right thing to do to start treating
32-bit filesystem seconds as being unsigned.  But from the commit message,
that didn't seem to be the intended effect.

The fix is simple; cast the result to (int) like XFS and ext2 do.
But someone needs to go through all the filesystems with care.  And it'd
be great if someone wrote an xfstest for handling files from 1968.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Files dated before 1970
  2020-06-20  2:16 Files dated before 1970 Matthew Wilcox
@ 2020-06-20  8:59 ` Arnd Bergmann
  2020-06-20 16:15   ` Matthew Wilcox
  2020-06-21 23:36   ` Dave Chinner
  0 siblings, 2 replies; 5+ messages in thread
From: Arnd Bergmann @ 2020-06-20  8:59 UTC (permalink / raw)
  To: Matthew Wilcox; +Cc: Deepa Dinamani, Linux FS-devel Mailing List

On Sat, Jun 20, 2020 at 4:16 AM Matthew Wilcox <willy@infradead.org> wrote:
>
>
> Hi Deepa,
>
> Your commit 95582b008388 ("vfs: change inode times to use struct
> timespec64") changed the behaviour of some filesystems with regards to
> files from before 1970.  Specifically, this line from JFS, unchanged
> since before 2.6.12:
>
> fs/jfs/jfs_imap.c:3065: ip->i_atime.tv_sec = le32_to_cpu(dip->di_atime.tv_sec);
>
> le32_to_cpu() returns a u32.  Before your patch, the u32 was assigned
> to an s32, so a file with a date stamp of 1968 would show up that way.
> After your patch, the u32 is zero-extended to an s64, so a file from
> 1968 now appears to be from 2104.
>
> Obviously there aren't a lot of files around from before 1970, so it's
> not surprising that nobody's noticed yet.  But I don't think this was
> an intended change.

In the case of JFS, I think the change of behavior on 32-bit kernels was
intended because it makes them do the same thing as 64-bit kernels.
I'm sure Deepa or I documented this somewhere but unfortunately it's
not clear from the commit description that actually made the transition :(.

> The fix is simple; cast the result to (int) like XFS and ext2 do.
> But someone needs to go through all the filesystems with care.  And it'd
> be great if someone wrote an xfstest for handling files from 1968.

I'm sure the xfstests check was added back when XFS and ext3 decided to
stick with the behavior of 32-bit kernels in order to avoid an
inadvertent change when 64-bit kernels originally became popular.

For JFS and the others that already used an unsigned interpretation
on 64 bit kernels, the current code seems to be the least broken
of the three alternatives we had:

a) as implemented in v4.18, change 32-bit kernels to behave the
   way that 64-bit kernels always have behaved, given that 99% of
   our users are on 64-bit kernels by now.

b) keep 32-bit and 64-bit kernels use a different interpretation,
   staying compatible with older kernels but incompatible between
   machines or between running the same user space on the
   same machine in either native 32-bit mode or compat mode
    a 64-bit kernel

c) change the 99% of users that have a 64-bit kernel to overflowing
    the timestamps in y2038 because that was what the kernel
    file system driver originally implemented on 32-bit machines
    that no concept of post-y2038 time.

    Arnd

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Files dated before 1970
  2020-06-20  8:59 ` Arnd Bergmann
@ 2020-06-20 16:15   ` Matthew Wilcox
  2020-06-21 23:38     ` Deepa Dinamani
  2020-06-21 23:36   ` Dave Chinner
  1 sibling, 1 reply; 5+ messages in thread
From: Matthew Wilcox @ 2020-06-20 16:15 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: Deepa Dinamani, Linux FS-devel Mailing List

On Sat, Jun 20, 2020 at 10:59:48AM +0200, Arnd Bergmann wrote:
> On Sat, Jun 20, 2020 at 4:16 AM Matthew Wilcox <willy@infradead.org> wrote:
> > le32_to_cpu() returns a u32.  Before your patch, the u32 was assigned
> > to an s32, so a file with a date stamp of 1968 would show up that way.
> > After your patch, the u32 is zero-extended to an s64, so a file from
> > 1968 now appears to be from 2104.
> 
> In the case of JFS, I think the change of behavior on 32-bit kernels was
> intended because it makes them do the same thing as 64-bit kernels.

Oh!  I hadn't realised that 64-bit kernels were already using a 64-bit
signed tv_sec.  That makes a world of difference.

> For JFS and the others that already used an unsigned interpretation
> on 64 bit kernels, the current code seems to be the least broken
> of the three alternatives we had:
> 
> a) as implemented in v4.18, change 32-bit kernels to behave the
>    way that 64-bit kernels always have behaved, given that 99% of
>    our users are on 64-bit kernels by now.
> 
> b) keep 32-bit and 64-bit kernels use a different interpretation,
>    staying compatible with older kernels but incompatible between
>    machines or between running the same user space on the
>    same machine in either native 32-bit mode or compat mode
>     a 64-bit kernel
> 
> c) change the 99% of users that have a 64-bit kernel to overflowing
>     the timestamps in y2038 because that was what the kernel
>     file system driver originally implemented on 32-bit machines
>     that no concept of post-y2038 time.

Yes, I agree, knowing more of the facts, this was the right decision to
make at the time, and I wouldn't change it now.  Thanks!

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Files dated before 1970
  2020-06-20  8:59 ` Arnd Bergmann
  2020-06-20 16:15   ` Matthew Wilcox
@ 2020-06-21 23:36   ` Dave Chinner
  1 sibling, 0 replies; 5+ messages in thread
From: Dave Chinner @ 2020-06-21 23:36 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: Matthew Wilcox, Deepa Dinamani, Linux FS-devel Mailing List

On Sat, Jun 20, 2020 at 10:59:48AM +0200, Arnd Bergmann wrote:
> On Sat, Jun 20, 2020 at 4:16 AM Matthew Wilcox <willy@infradead.org> wrote:
> >
> >
> > Hi Deepa,
> >
> > Your commit 95582b008388 ("vfs: change inode times to use struct
> > timespec64") changed the behaviour of some filesystems with regards to
> > files from before 1970.  Specifically, this line from JFS, unchanged
> > since before 2.6.12:
> >
> > fs/jfs/jfs_imap.c:3065: ip->i_atime.tv_sec = le32_to_cpu(dip->di_atime.tv_sec);
> >
> > le32_to_cpu() returns a u32.  Before your patch, the u32 was assigned
> > to an s32, so a file with a date stamp of 1968 would show up that way.
> > After your patch, the u32 is zero-extended to an s64, so a file from
> > 1968 now appears to be from 2104.
> >
> > Obviously there aren't a lot of files around from before 1970, so it's
> > not surprising that nobody's noticed yet.  But I don't think this was
> > an intended change.
> 
> In the case of JFS, I think the change of behavior on 32-bit kernels was
> intended because it makes them do the same thing as 64-bit kernels.
> I'm sure Deepa or I documented this somewhere but unfortunately it's
> not clear from the commit description that actually made the transition :(.
> 
> > The fix is simple; cast the result to (int) like XFS and ext2 do.
> > But someone needs to go through all the filesystems with care.  And it'd
> > be great if someone wrote an xfstest for handling files from 1968.
> 
> I'm sure the xfstests check was added back when XFS and ext3 decided to
> stick with the behavior of 32-bit kernels in order to avoid an
> inadvertent change when 64-bit kernels originally became popular.


$ ./lsqa.pl tests/generic/258

FS QA Test No. 258

Test timestamps prior to epoch
On 64-bit, ext2/3/4 was sign-extending when read from disk
See also commit 4d7bf11d649c72621ca31b8ea12b9c94af380e63

$

Commit 4d7bf11d649c ("ext2/3/4: fix file date underflow on ext2 3
filesystems on 64 bit systems") was indeed as you describe, but it
was only ext2/3/4 that had this problem. XFS has always had
consistent behaviour of timestamps across 32 and 64 bit systems
because that's what it inherited from Irix. :)

FWIW, this test was written in 2011, long before the y2038k work
started.  Hence if the filesystem does not pass generic/258, it's a
fair bet that JFS was broken on 64 bit kernels right from the
start....

Hmmm - generic/258 runs on filesystems where:

_require_negative_timestamps

does not abort the test.

Which, according to common/rc:

# exfat timestamps start at 1980 and cannot be prior to epoch
_require_negative_timestamps() {
        case "$FSTYP" in
        ceph|exfat)
                _notrun "$FSTYP does not support negative timestamps"
                ;;
        esac
}

Is every supported filesystem except exfat and ceph. Hence the list
of supported filesystems include:

	NFS, Glusterfs, CIFS, 9p, virtiofs, overlay, pvfs2, tmpfs
	ubifs, btrfs, ext2/3/4, XFS, JFS, reiserfs, reiser4, Btrfs,
	ocfs2, udf, gfs2, f2fs

So, really, if this is a long standing bug in the 64bit JFS
implementation in that it doesn't support negative timestamps, it
tells us that either nobody has been running fstests on 64-bit JFS
for almost 10 years or they are ignoring long standing failures.
Either way, it indicates that JFS really isn't being maintained by
anyone.

> For JFS and the others that already used an unsigned interpretation
> on 64 bit kernels, the current code seems to be the least broken
> of the three alternatives we had:

For anything that is actively maintained, it should be fixed to
pass generic/258 or have an exception added to
_require_negative_timestamps().

For filesystems that are not actively maintained, should we use this
as a sign we should seriously consider removing those filesystems
from the upstream tree?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Files dated before 1970
  2020-06-20 16:15   ` Matthew Wilcox
@ 2020-06-21 23:38     ` Deepa Dinamani
  0 siblings, 0 replies; 5+ messages in thread
From: Deepa Dinamani @ 2020-06-21 23:38 UTC (permalink / raw)
  To: Matthew Wilcox; +Cc: Arnd Bergmann, Linux FS-devel Mailing List

I included a table of the time ranges supported as part of another
series: https://lkml.org/lkml/2019/8/18/169.
There had been some discussions with individual file system
maintainers about what was the file system's intended range of
supported timestamps.
Eg: https://patchwork.kernel.org/patch/8308691/.

I also added an xfs
test(https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/tree/tests/generic/402)
to test the ranges supported. But, since there was no way to query the
kernel for ranges at that time, the ranges have to be plugged into
xfstests for this to work on all filesystems. This does check for
dates before 1970 if the range allows it.

-Deepa

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-06-21 23:38 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-20  2:16 Files dated before 1970 Matthew Wilcox
2020-06-20  8:59 ` Arnd Bergmann
2020-06-20 16:15   ` Matthew Wilcox
2020-06-21 23:38     ` Deepa Dinamani
2020-06-21 23:36   ` Dave Chinner

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.