diskfs / go-diskfs

MIT License
494 stars 112 forks source link

Wrong timestamps for special FAT-32 directory entries #212

Closed max-gotlib closed 4 months ago

max-gotlib commented 4 months ago

Symptoms:

This problem prevents some scripting working in the grub config files.

The grub loader has strict rules for the timestamps (<grub-2.12>/include/grub/datetime.h:54):

static inline int
grub_datetime2unixtime (const struct grub_datetime *datetime, grub_int64_t *nix)
{
  ...
  if (datetime->year > 2038 || datetime->year < 1901)
    return 0;
  if (datetime->month > 12 || datetime->month < 1)
    return 0;
  ...
}

What's generated with go-diskfs:

Apparently, the 0x8a21 value corresponds to 2049/01/01 date (according to the FAT rules) which is far in the future related to the image file generation date (2024 May 5).

Possible fix (verified, makes grub happy):

diff --git a/go-diskfs/filesystem/fat32/fat32.go b/go-diskfs/filesystem/fat32/fat32.go
index 84961587..c317e813 100644
--- a/vendor/github.com/diskfs/go-diskfs/filesystem/fat32/fat32.go
+++ b/vendor/github.com/diskfs/go-diskfs/filesystem/fat32/fat32.go
@@ -850,6 +850,7 @@ func (fs *FileSystem) readDirWithMkdir(p string, doMake bool) (*Directory, []*di
                if err != nil {
                    return nil, nil, fmt.Errorf("failed to create subdirectory %s", "/"+strings.Join(paths[0:i+1], "/"))
                }
+               currentDir.modifyTime = subdirEntry.createTime
                // make a basic entry for the new subdir
                parentDirectoryCluster := currentDir.clusterLocation
                if parentDirectoryCluster == 2 {
@@ -859,8 +860,22 @@ func (fs *FileSystem) readDirWithMkdir(p string, doMake bool) (*Directory, []*di
                dir := &Directory{
                    directoryEntry: directoryEntry{clusterLocation: subdirEntry.clusterLocation},
                    entries: []*directoryEntry{
-                       {filenameShort: ".", isSubdirectory: true, clusterLocation: subdirEntry.clusterLocation},
-                       {filenameShort: "..", isSubdirectory: true, clusterLocation: parentDirectoryCluster},
+                       {
+                           filenameShort:   ".",
+                           isSubdirectory:  true,
+                           clusterLocation: subdirEntry.clusterLocation,
+                           createTime:      subdirEntry.createTime,
+                           modifyTime:      subdirEntry.modifyTime,
+                           accessTime:      subdirEntry.accessTime,
+                       },
+                       {
+                           filenameShort:   "..",
+                           isSubdirectory:  true,
+                           clusterLocation: parentDirectoryCluster,
+                           createTime:      currentDir.createTime,
+                           modifyTime:      currentDir.modifyTime,
+                           accessTime:      currentDir.accessTime,
+                       },
                    },
                }
                // write the new directory entries to disk
deitch commented 4 months ago

This is one impressive issue. Screen caps and detailed hex-level description. Thank you for doing the detailed work.

deitch commented 4 months ago

If I understood the issue correctly, grub is erroring when reading the special directory entries "." and "..", the first ones in any directory created. The lines you referenced here are, indeed, where those are created (and the only such place in fat32).

The directory entry is turned into bytes to be written to the filesystem here, with the important parts being:

    createDate, createTime := timeToDateTime(de.createTime)
    modifyDate, modifyTime := timeToDateTime(de.modifyTime)
    accessDate, _ := timeToDateTime(de.accessTime)
    binary.LittleEndian.PutUint16(dosBytes[14:16], createTime)
    binary.LittleEndian.PutUint16(dosBytes[16:18], createDate)
    binary.LittleEndian.PutUint16(dosBytes[18:20], accessDate)
    binary.LittleEndian.PutUint16(dosBytes[22:24], modifyTime)
``

So the key is `timeToDateTime()`:

```go
func timeToDateTime(t time.Time) (datePart, timePart uint16) {
    year := t.Year()
    month := int(t.Month())
    day := t.Day()
    second := t.Second()
    minute := t.Minute()
    hour := t.Hour()
    retDate := (year-1980)<<9 + (month << 5) + day
    retTime := hour<<11 + minute<<5 + (second / 2)
    return uint16(retDate), uint16(retTime)
}

I ran a quick test of timeToDateTime(t) where t is the default time.Time (i.e. the case for the new entries, and we get time: 0001-01-01 00:00:00 +0000 UTC, outDate 35361, outTime 0, outDate is 35361, or 0x8A21, exactly what you found, and hence the source of the problem. The relevant logic for date is:

    year := t.Year()
    month := int(t.Month())
    day := t.Day()
    retDate := (year-1980)<<9 + (month << 5) + day

In the 0 case, that becomes year, month, date = 1, 1, 1, or 1 January 1. That then turns into -1013215 by the last line, and, when converted to uint16, becomes 35361. Clearly, the FAT32 logic was not built to handle dates of 1970 years before the epoch!

I see a few possible fixes:

I think both of these are correct, but for the immediate issue, your approach is the correct one.

Do you want to open a Pull Request with it?

max-gotlib commented 4 months ago

Do you want to open a Pull Request with it?

Yep. Onto it.