From 431db66aae07cf3d1d35ea87c087124335d3d395 Mon Sep 17 00:00:00 2001 From: Simon Ruderich Date: Sun, 15 Jan 2023 10:58:36 +0100 Subject: [PATCH] remote: add and improve comments --- remote/ainsl/ainsl.go | 2 +- remote/sync/files.go | 33 ++++++++++++++++++--------------- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/remote/ainsl/ainsl.go b/remote/ainsl/ainsl.go index 8b4aa88..e4699b8 100644 --- a/remote/ainsl/ainsl.go +++ b/remote/ainsl/ainsl.go @@ -161,7 +161,7 @@ func handle(path string, line string, create bool) ([]string, error) { } err = unix.Renameat(parentFd, tmpBase, parentFd, baseName) if err != nil { - unix.Unlinkat(parentFd, tmpBase, 0) //nolint:errcheck + unix.Unlinkat(parentFd, tmpBase, 0 /* flags */) //nolint:errcheck return nil, err } err = unix.Fsync(parentFd) diff --git a/remote/sync/files.go b/remote/sync/files.go index 56c9ff0..98c4ad6 100644 --- a/remote/sync/files.go +++ b/remote/sync/files.go @@ -49,7 +49,7 @@ import ( // openReadonlyFlags are flags for open* syscalls to safely read a file or // directory. // -// O_NOFOLLOW prevents symlink attacks +// O_NOFOLLOW prevents symlink attacks in the last path component // O_NONBLOCK is necessary to prevent blocking on FIFOs const openReadonlyFlags = unix.O_RDONLY | unix.O_NOFOLLOW | unix.O_NONBLOCK @@ -83,13 +83,13 @@ func (s *Sync) syncFiles() error { func (s *Sync) syncFile(file *safcm.File, changed *bool) error { // The general strategy is "update by rename": If any property of a - // file changes it will be written to a temporary file and then - // renamed "over" the original file. This is simple and prevents race - // conditions where the file is partially readable while changes to - // permissions or owner/group are applied. However, this strategy does - // not work for directories which must be removed first (was - // directory), must remove the existing file (will be directory) or - // must be directly modified (changed permissions or owner). In the + // file changes the new version will be written to a temporary file + // and then renamed "over" the original file. This is simple and + // prevents race conditions where the file is partially readable while + // changes to permissions or owner/group are applied. However, this + // strategy does not work for directories which must be removed first + // (was directory), must remove the existing file (will be directory) + // or must be directly modified (changed permissions or owner). In the // first two cases the old path is removed. In the last the directory // is modified (carefully) in place. // @@ -219,7 +219,9 @@ reopen: // Some BSD systems permit changing permissions of // symlinks but ignore them on traversal. To keep it // simple we don't support that and always use 0777 - // for symlink permissions (the value on GNU/Linux). + // for symlink permissions (the value on GNU/Linux) + // when comparing. The actual permissions on the file + // system might be different on BSD systems. // // TODO: Add proper support for symlinks on BSD change.Old.Mode |= 0777 @@ -330,7 +332,7 @@ reopen: // (accidentally) replacing a directory tree with a file. const msg = "will not replace non-empty directory, " + "please remove manually" - err := unix.Unlinkat(parentFd, baseName, 0) + err := unix.Unlinkat(parentFd, baseName, 0 /* flags */) if err != nil && !os.IsNotExist(err) { err2 := unix.Unlinkat(parentFd, baseName, unix.AT_REMOVEDIR) @@ -454,7 +456,7 @@ reopen: err = unix.Fchownat(parentFd, tmpBase, file.Uid, file.Gid, unix.AT_SYMLINK_NOFOLLOW) if err != nil { - unix.Unlinkat(parentFd, tmpBase, 0) //nolint:errcheck + unix.Unlinkat(parentFd, tmpBase, 0 /* flags */) //nolint:errcheck return err } // Permissions are irrelevant for symlinks (on most systems) @@ -466,7 +468,7 @@ reopen: debugf("renaming %q", slashpath.Join(dir, tmpBase)) err = unix.Renameat(parentFd, tmpBase, parentFd, baseName) if err != nil { - unix.Unlinkat(parentFd, tmpBase, 0) //nolint:errcheck + unix.Unlinkat(parentFd, tmpBase, 0 /* flags */) //nolint:errcheck return err } // To guarantee durability fsync must be called on a parent directory @@ -569,7 +571,8 @@ func OpenParentDirectoryNoSymlinks(path string) (int, string, error) { } } - dirFd, err := unix.Openat(unix.AT_FDCWD, dir, openReadonlyFlags, 0) + dirFd, err := unix.Openat(unix.AT_FDCWD, dir, + openReadonlyFlags, 0 /* mode */) if err != nil { return -1, "", err } @@ -649,7 +652,7 @@ func OpenFileNoSymlinks(path string) (*os.File, error) { } func OpenAtNoFollow(dirFd int, base string) (*os.File, error) { - fd, err := unix.Openat(dirFd, base, openReadonlyFlags, 0) + fd, err := unix.Openat(dirFd, base, openReadonlyFlags, 0 /* mode */) if err != nil { return nil, err } @@ -667,7 +670,7 @@ func WriteTempAt(dirFd int, base string, data []byte, uid, gid int, _, err = fh.Write(data) if err != nil { fh.Close() - unix.Unlinkat(dirFd, tmpBase, 0) //nolint:errcheck + unix.Unlinkat(dirFd, tmpBase, 0 /* flags */) //nolint:errcheck return "", err } // createTempAt() creates the file with 0600 -- 2.45.2