-
Notifications
You must be signed in to change notification settings - Fork 204
Description
Code that calls SetFileBasicInfo(f *os.File, bi *FileBasicInfo) error can in some cases get an error like this:
SetFileInformationByHandle \\?\C:\Windows\SystemTemp\hcs2001018820\Files.$wcidirs$: Invalid access to memory location.
This is ERROR_NOACCESS, which can indicate a STATUS_DATATYPE_MISALIGNMENT error.
The reason is that winio's FileBasicInfo uses windows.Filetime, causing the struct alignment to be wrong. I can't directly prove the cause for sure (it's not trivial to repro with specific functions), but I opened golang/go#65069 to get some help with it, and I don't think there's any doubt.
Here's FileBasicInfo for reference:
Lines 14 to 19 in bc421d9
| // FileBasicInfo contains file access time and file attributes information. | |
| type FileBasicInfo struct { | |
| CreationTime, LastAccessTime, LastWriteTime, ChangeTime windows.Filetime | |
| FileAttributes uint32 | |
| _ uint32 // padding | |
| } |
type Filetime struct {
LowDateTime uint32
HighDateTime uint32
}The biggest field of FileBasicInfo is uint32, so the struct as a whole is aligned on 32 bits.
But the input to SetFileInformationByHandle needs to be 64-bit aligned to be compatible with Windows' struct:
FileBasicInfo defines the time properties using
LARGE_INTEGER, which is auint64.
typedef struct _FILE_BASIC_INFORMATION {
LARGE_INTEGER CreationTime;
LARGE_INTEGER LastAccessTime;
LARGE_INTEGER LastWriteTime;
LARGE_INTEGER ChangeTime;
ULONG FileAttributes;
} FILE_BASIC_INFORMATION, *PFILE_BASIC_INFORMATION;
A workaround is to define a new struct with the right alignment and copy the data over before sending it to the syscall:
func SetFileBasicInfo(f *os.File, bi *FileBasicInfo) error {
merge := func(t windows.Filetime) uint64 {
return *(*uint64)(unsafe.Pointer(&t))
}
biAligned := struct {
CreationTime, LastAccessTime, LastWriteTime, ChangeTime uint64
FileAttributes uint32
_ uint32 // padding
}{
merge(bi.CreationTime), merge(bi.LastAccessTime), merge(bi.LastWriteTime), merge(bi.ChangeTime),
bi.FileAttributes,
0,
}
if err := windows.SetFileInformationByHandle(
windows.Handle(f.Fd()),
windows.FileBasicInfo,
(*byte)(unsafe.Pointer(&biAligned)),
uint32(unsafe.Sizeof(biAligned)),
); err != nil {
return &os.PathError{Op: "SetFileInformationByHandle", Path: f.Name(), Err: err}
}
runtime.KeepAlive(f)
return nil
}It seems to me that something similar should also be added to GetFileBasicInfo. At least, I don't know any reason why the alignment problem wouldn't also eventually cause a problem for someone using that function, too.
This issue is making moby/moby's attempt to update to go1.22rc1 fail in dockerd: moby/moby#46982. I confirmed that applying the workaround above makes the update pass CI.
For some reason, go1.22rc1 aggravated this issue in moby's case, but I don't see any reason it wouldn't affect any Go version, depending on your luck. More info at golang/go#65069.
I wrote a test case that could make it easier to spot this kind of thing in PR review: https://gist.github.com/dagood/3cd4207c1f12608f1cca2183af143715.