Bug 210997 - Month timestamp metadata advanced by one by exfat staging driver
Summary: Month timestamp metadata advanced by one by exfat staging driver
Status: RESOLVED CODE_FIX
Alias: None
Product: File System
Classification: Unclassified
Component: Other (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: fs_other
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-01-01 23:43 UTC by Arpad Müller
Modified: 2021-02-20 11:57 UTC (History)
1 user (show)

See Also:
Kernel Version: 5.4
Subsystem:
Regression: No
Bisected commit-id:


Attachments

Description Arpad Müller 2021-01-01 23:43:11 UTC
Reproduction:

# A) Create an exfat formatted device.
dd if=/dev/zero of=exfat.bin bs=10M count=1
mkfs.exfat exfat.bin

# B) Mount the device.
mkdir /tmp/foo
sudo mount -t exfat exfat.bin /tmp/foo

# C) Ensure that the kernel exfat driver is used instead of the fuse driver.
# I had to remove the exfat package from my distro to make this happen.
# It should print:
# $ mount | grep "type exfat"
# /path/to/exfat.bin on /tmp/foo type exfat (rw,relatime,fmask=0022,dmask=0022,iocharset=utf8,namecase=0,errors=remount-ro)

# D) Create a test file and verify that the current date is shown.
sudo touch /tmp/foo/a
stat /tmp/foo/a
# Shows current date

# E) Bug reproduction: unmount the device and re-mount it.
sudo umount /tmp/foo
sudo mount -t exfat exfat.bin /tmp/foo
stat /tmp/foo/a
# Shows date one month into the future!!

# F) This offset can be increased by simple access of the file:
cat /tmp/foo/a
sudo umount /tmp/foo
sudo mount -t exfat exfat.bin /tmp/foo
stat /tmp/foo/a
# Shows date *two* months into the future!!
# Repeat step F) to get dates arbitrarily far into the future

I have stumbled across this bug when I visited my family over Christmas and
read some pictures from my dad's SD card. Little later when he copied over
pictures to his Windows PC he pointed out the issue of timestamps being
corrupt to me. All I did was copy some pictures over, I didn't change anything.
I've since then confirmed that reading the file and thus bumping the atime
alone is enough to trigger the bug and corrupt the timestamp values in the
filesystem. As I'd mounted the device several times, the timestamps (changed,
created, etc) were advanced by multiple months into the future.

The launchpad report [0] that I found a little bit later contains the cause of
the bug: the exfat_time_fat2unix function contains a call to the mktime64
function where it adds 1 to the month value [3]. This is wrong though as the
month is 1-based in both the disk format [2] as well as the format that mktime64
takes in. Maybe the author was confused because struct tm *does* work with 0
based months, so in the other instances in that file appending 1 to the month is
justified.

This bug only exists in the old staging driver of the exfat file system. The
driver in fs/exfat/ is immune, as is the fuse driver. The staging driver shipped
with the 5.4 release, which is an LTS release. Thus, I believe this bug is still
relevant, as the release will continue to see usage for years, and deployed
systems will continue to corrupt timestamps. Again, on newer kernels (like 5.10)
that use the fs/exfat/ driver, the bug doesn't exist.

For a fix, I've tried removing the +1 at the described location. After that 
I couldn't reproduce the issue any more. So it should be a simple one liner 
patch. I've wondered about submitting it directly instead of filing this report,
but it's not me who has tracked down the bug, but Sebasitan Gürtler in the 
Launchpad issue. Just submitting the patch without his permission seemed wrong 
to me.

[0]: https://bugs.launchpad.net/ubuntu/+source/ubuntu-meta/+bug/1872504

[1]: https://github.com/torvalds/linux/blob/v5.4/drivers/staging/exfat/exfat_super.c#L62

[2]: https://docs.microsoft.com/en-gb/windows/win32/fileio/exfat-specification#7485-month-field

[3]: https://github.com/torvalds/linux/blob/v5.4/kernel/time/time.c#L417
Comment 1 Arpad Müller 2021-01-13 10:26:47 UTC
> I've wondered about submitting it directly instead of filing this report,
but it's not me who has tracked down the bug, but Sebasitan Gürtler in the 
Launchpad issue. Just submitting the patch without his permission seemed wrong 
to me.

I've sent him an E-Mail 2 weeks ago (on Dec 29) about the issue, asking whether he wanted to submit the patch himself. But I've gotten no reply since. Maybe I should just submit the patch anyways? Ideally the bug gets fixed sooner rather than later :).
Comment 2 Arpad Müller 2021-02-20 11:57:00 UTC
With the 5.4.90 release a fix has landed in the 5.4 LTS branch, so this issue is resolved. Testing confirms that the bug isn't reproducible any more. Thanks.

Note You need to log in before you can comment on or make changes to this bug.