From af4c1fb7a3f428ff704af22294ad955d05ed41dd Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Wed, 14 Oct 2020 00:35:16 +0200 Subject: [PATCH] syscallcompat: retry ops on EINTR Retry operations that have been shown to throw EINTR errors on CIFS. Todo: Solution for this pain in the back: warning: unix.Getdents returned errno 2 in the middle of data rm: cannot remove 'linux-3.0.old3/Documentation/ABI/removed': Input/output error Progress towards fixing https://github.com/rfjakob/gocryptfs/issues/483 . --- internal/fusefrontend/file.go | 2 +- internal/fusefrontend/node_dir_ops.go | 2 +- internal/fusefrontend/root_node.go | 4 ++- internal/nametransform/diriv.go | 1 + internal/syscallcompat/eintr.go | 48 +++++++++++++++++++++++++ internal/syscallcompat/open_nofollow.go | 5 ++- internal/syscallcompat/sys_common.go | 34 +++++++++++------- internal/syscallcompat/sys_linux.go | 12 +++++-- 8 files changed, 90 insertions(+), 18 deletions(-) create mode 100644 internal/syscallcompat/eintr.go diff --git a/internal/fusefrontend/file.go b/internal/fusefrontend/file.go index 66b3e24..0367705 100644 --- a/internal/fusefrontend/file.go +++ b/internal/fusefrontend/file.go @@ -409,7 +409,7 @@ func (f *File) Flush(ctx context.Context) syscall.Errno { if err != nil { return fs.ToErrno(err) } - err = syscall.Close(newFd) + err = syscallcompat.Close(newFd) return fs.ToErrno(err) } diff --git a/internal/fusefrontend/node_dir_ops.go b/internal/fusefrontend/node_dir_ops.go index a93271d..b8d4ec7 100644 --- a/internal/fusefrontend/node_dir_ops.go +++ b/internal/fusefrontend/node_dir_ops.go @@ -306,7 +306,7 @@ retry: return fs.ToErrno(err) } if err != nil { - tlog.Warn.Printf("Rmdir: Readdirnames: %v", err) + tlog.Warn.Printf("Rmdir: Getdents: %v", err) return fs.ToErrno(err) } // MacOS sprinkles .DS_Store files everywhere. This is hard to avoid for diff --git a/internal/fusefrontend/root_node.go b/internal/fusefrontend/root_node.go index 5870c97..e03e250 100644 --- a/internal/fusefrontend/root_node.go +++ b/internal/fusefrontend/root_node.go @@ -203,6 +203,8 @@ func (rn *RootNode) openWriteOnlyFile(dirfd int, cName string, newFlags int) (rw // // openBackingDir is secure against symlink races by using Openat and // ReadDirIVAt. +// +// Retries on EINTR. func (rn *RootNode) openBackingDir(relPath string) (dirfd int, cName string, err error) { dirRelPath := nametransform.Dir(relPath) // With PlaintextNames, we don't need to read DirIVs. Easy. @@ -216,7 +218,7 @@ func (rn *RootNode) openBackingDir(relPath string) (dirfd int, cName string, err return dirfd, cName, nil } // Open cipherdir (following symlinks) - dirfd, err = syscall.Open(rn.args.Cipherdir, syscall.O_DIRECTORY|syscallcompat.O_PATH, 0) + dirfd, err = syscallcompat.Open(rn.args.Cipherdir, syscall.O_DIRECTORY|syscallcompat.O_PATH, 0) if err != nil { return -1, "", err } diff --git a/internal/nametransform/diriv.go b/internal/nametransform/diriv.go index cd20ca6..6dbf028 100644 --- a/internal/nametransform/diriv.go +++ b/internal/nametransform/diriv.go @@ -23,6 +23,7 @@ const ( // ReadDirIVAt reads "gocryptfs.diriv" from the directory that is opened as "dirfd". // Using the dirfd makes it immune to concurrent renames of the directory. +// Retries on EINTR. func ReadDirIVAt(dirfd int) (iv []byte, err error) { fdRaw, err := syscallcompat.Openat(dirfd, DirIVFilename, syscall.O_RDONLY|syscall.O_NOFOLLOW, 0) diff --git a/internal/syscallcompat/eintr.go b/internal/syscallcompat/eintr.go new file mode 100644 index 0000000..122af9b --- /dev/null +++ b/internal/syscallcompat/eintr.go @@ -0,0 +1,48 @@ +package syscallcompat + +import ( + "syscall" +) + +// retryEINTR executes operation `op` and retries if it gets EINTR. +// +// Like ignoringEINTR() in the Go stdlib: +// https://github.com/golang/go/blob/d2a80f3fb5b44450e0b304ac5a718f99c053d82a/src/os/file_posix.go#L243 +// +// This is needed because CIFS throws lots of EINTR errors: +// https://github.com/rfjakob/gocryptfs/issues/483 +func retryEINTR(op func() error) error { + for { + err := op() + if err != syscall.EINTR { + return err + } + } +} + +// retryEINTR2 is like retryEINTR but for functions that return an (int, error) +// pair like syscall.Create(). +func retryEINTR2(op func() (int, error)) (int, error) { + for { + ret, err := op() + if err != syscall.EINTR { + return ret, err + } + } +} + +// Open wraps syscall.Open. +// Retries on EINTR. +func Open(path string, mode int, perm uint32) (fd int, err error) { + fd, err = retryEINTR2(func() (int, error) { + return syscall.Open(path, mode, perm) + }) + return fd, err +} + +// Close wraps syscall.Close. +// Retries on EINTR. +func Close(fd int) (err error) { + err = retryEINTR(func() error { return syscall.Close(fd) }) + return err +} diff --git a/internal/syscallcompat/open_nofollow.go b/internal/syscallcompat/open_nofollow.go index a1e7cce..f8e50e3 100644 --- a/internal/syscallcompat/open_nofollow.go +++ b/internal/syscallcompat/open_nofollow.go @@ -13,6 +13,7 @@ import ( // This function is implemented by walking the directory tree, starting at // "baseDir", using the Openat syscall with the O_NOFOLLOW flag. // Symlinks that are part of the "baseDir" path are followed. +// Retries on EINTR. func OpenDirNofollow(baseDir string, relPath string) (fd int, err error) { if !filepath.IsAbs(baseDir) { tlog.Warn.Printf("BUG: OpenDirNofollow called with relative baseDir=%q", baseDir) @@ -23,7 +24,9 @@ func OpenDirNofollow(baseDir string, relPath string) (fd int, err error) { return -1, syscall.EINVAL } // Open the base dir (following symlinks) - dirfd, err := syscall.Open(baseDir, syscall.O_DIRECTORY|O_PATH, 0) + dirfd, err := retryEINTR2(func() (int, error) { + return syscall.Open(baseDir, syscall.O_DIRECTORY|O_PATH, 0) + }) if err != nil { return -1, err } diff --git a/internal/syscallcompat/sys_common.go b/internal/syscallcompat/sys_common.go index c50c940..d178a9b 100644 --- a/internal/syscallcompat/sys_common.go +++ b/internal/syscallcompat/sys_common.go @@ -44,7 +44,8 @@ func Faccessat(dirfd int, path string, mode uint32) error { return unix.Faccessat(dirfd, path, mode, 0) } -// Openat wraps the Openat syscall, retrying on EINTR. +// Openat wraps the Openat syscall. +// Retries on EINTR. func Openat(dirfd int, path string, flags int, mode uint32) (fd int, err error) { if flags&syscall.O_CREAT != 0 { // O_CREAT should be used with O_EXCL. O_NOFOLLOW has no effect with O_EXCL. @@ -59,24 +60,28 @@ func Openat(dirfd int, path string, flags int, mode uint32) (fd int, err error) flags |= syscall.O_NOFOLLOW } } - // Like ignoringEINTR() in the Go stdlib: - // https://github.com/golang/go/blob/d2a80f3fb5b44450e0b304ac5a718f99c053d82a/src/os/file_posix.go#L243 - for { - fd, err = unix.Openat(dirfd, path, flags, mode) - if err != unix.EINTR { - return fd, err - } - } + fd, err = retryEINTR2(func() (int, error) { + return unix.Openat(dirfd, path, flags, mode) + }) + return fd, err } // Renameat wraps the Renameat syscall. +// Retries on EINTR. func Renameat(olddirfd int, oldpath string, newdirfd int, newpath string) (err error) { - return unix.Renameat(olddirfd, oldpath, newdirfd, newpath) + err = retryEINTR(func() error { + return unix.Renameat(olddirfd, oldpath, newdirfd, newpath) + }) + return err } // Unlinkat syscall. +// Retries on EINTR. func Unlinkat(dirfd int, path string, flags int) (err error) { - return unix.Unlinkat(dirfd, path, flags) + err = retryEINTR(func() error { + return unix.Unlinkat(dirfd, path, flags) + }) + return err } // Fchownat syscall. @@ -105,17 +110,22 @@ func Mkdirat(dirfd int, path string, mode uint32) (err error) { } // Fstatat syscall. +// Retries on EINTR. func Fstatat(dirfd int, path string, stat *unix.Stat_t, flags int) (err error) { // Why would we ever want to call this without AT_SYMLINK_NOFOLLOW? if flags&unix.AT_SYMLINK_NOFOLLOW == 0 { tlog.Warn.Printf("Fstatat: adding missing AT_SYMLINK_NOFOLLOW flag") flags |= unix.AT_SYMLINK_NOFOLLOW } - return unix.Fstatat(dirfd, path, stat, flags) + err = retryEINTR(func() error { + return unix.Fstatat(dirfd, path, stat, flags) + }) + return err } // Fstatat2 is a more convenient version of Fstatat. It allocates a Stat_t // for you and also handles the Unix2syscall conversion. +// Retries on EINTR. func Fstatat2(dirfd int, path string, flags int) (*syscall.Stat_t, error) { var stUnix unix.Stat_t err := Fstatat(dirfd, path, &stUnix, flags) diff --git a/internal/syscallcompat/sys_linux.go b/internal/syscallcompat/sys_linux.go index 6273504..06b9696 100644 --- a/internal/syscallcompat/sys_linux.go +++ b/internal/syscallcompat/sys_linux.go @@ -265,9 +265,13 @@ func FutimesNano(fd int, a *time.Time, m *time.Time) (err error) { } // UtimesNanoAtNofollow is like UtimesNanoAt but never follows symlinks. +// Retries on EINTR. func UtimesNanoAtNofollow(dirfd int, path string, a *time.Time, m *time.Time) (err error) { ts := timesToTimespec(a, m) - return unix.UtimesNanoAt(dirfd, path, ts, unix.AT_SYMLINK_NOFOLLOW) + err = retryEINTR(func() error { + return unix.UtimesNanoAt(dirfd, path, ts, unix.AT_SYMLINK_NOFOLLOW) + }) + return err } // Getdents syscall. @@ -276,6 +280,10 @@ func Getdents(fd int) ([]fuse.DirEntry, error) { } // Renameat2 does not exist on Darwin, so we have to wrap it here. +// Retries on EINTR. func Renameat2(olddirfd int, oldpath string, newdirfd int, newpath string, flags uint) (err error) { - return unix.Renameat2(olddirfd, oldpath, newdirfd, newpath, flags) + err = retryEINTR(func() error { + return unix.Renameat2(olddirfd, oldpath, newdirfd, newpath, flags) + }) + return err }