diff options
| author | Jakob Unterwurzacher | 2020-10-14 00:35:16 +0200 | 
|---|---|---|
| committer | Jakob Unterwurzacher | 2020-10-14 00:35:16 +0200 | 
| commit | af4c1fb7a3f428ff704af22294ad955d05ed41dd (patch) | |
| tree | 66bdf7480ff1dd82bf2653c2070871762f24d742 | |
| parent | 803fdf410bb57d34ef09357ec4924646978c20b5 (diff) | |
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 .
| -rw-r--r-- | internal/fusefrontend/file.go | 2 | ||||
| -rw-r--r-- | internal/fusefrontend/node_dir_ops.go | 2 | ||||
| -rw-r--r-- | internal/fusefrontend/root_node.go | 4 | ||||
| -rw-r--r-- | internal/nametransform/diriv.go | 1 | ||||
| -rw-r--r-- | internal/syscallcompat/eintr.go | 48 | ||||
| -rw-r--r-- | internal/syscallcompat/open_nofollow.go | 5 | ||||
| -rw-r--r-- | internal/syscallcompat/sys_common.go | 34 | ||||
| -rw-r--r-- | internal/syscallcompat/sys_linux.go | 12 | 
8 files changed, 90 insertions, 18 deletions
| 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  } | 
