From 2cdd39bd6129c6a49c74fb07fb9d77ba1271c572 Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Tue, 17 Nov 2015 01:24:26 -0500 Subject: [PATCH] kqueue: avoid infinite loops from symlinks cycles The semenatics of using kqueue are already slightly different than inotify, specifically that inotify will give you events for the symlink itself whereas kqueue will only give events for the target of the symlink. This preserves that behaviour, that symlinks don't get notified on, but ensures that circular symlinks don't recurse forever. This change pushes the resolved filename up through the stack so that the fileExists map can be properly populated; not doing so would mean that deleting a cyclical symlink and then replacing it with a file of the same name would not notify for that file's creation (or subsequent events) because we would believe that the file already existed. --- AUTHORS | 1 + CHANGELOG.md | 3 ++- integration_test.go | 48 +++++++++++++++++++++++++++++++++++++++++++++ kqueue.go | 43 ++++++++++++++++++++++++++-------------- 4 files changed, 79 insertions(+), 16 deletions(-) diff --git a/AUTHORS b/AUTHORS index 05b3f3e..763b853 100644 --- a/AUTHORS +++ b/AUTHORS @@ -13,6 +13,7 @@ Caleb Spare Case Nelson Chris Howey Christoffer Buchholz +Daniel Wagner-Hall Dave Cheney Evan Phoenix Francisco Souza diff --git a/CHANGELOG.md b/CHANGELOG.md index 208bcd7..4e66727 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,10 +1,11 @@ # Changelog -## v1.2.4 / 2015-10-17 +## v1.2.5 / 2015-10-17 * inotify: use epoll_create1 for arm64 support (requires Linux 2.6.27 or later) [#100](https://github.com/go-fsnotify/fsnotify/pull/100) (thanks @suihkulokki) * inotify: fix path leaks [#73](https://github.com/go-fsnotify/fsnotify/pull/73) (thanks @chamaken) * kqueue: watch for rename events on subdirectories [#83](https://github.com/go-fsnotify/fsnotify/pull/83) (thanks @guotie) +* kqueue: avoid infinite loops from symlinks cycles [#101](https://github.com/go-fsnotify/fsnotify/pull/101) (thanks @illicitonion) ## v1.2.1 / 2015-10-14 diff --git a/integration_test.go b/integration_test.go index 59169c6..49e48ff 100644 --- a/integration_test.go +++ b/integration_test.go @@ -10,6 +10,7 @@ import ( "io/ioutil" "os" "os/exec" + "path" "path/filepath" "runtime" "sync/atomic" @@ -1065,6 +1066,53 @@ func TestFsnotifyFakeSymlink(t *testing.T) { watcher.Close() } +func TestCyclicSymlink(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("symlinks don't work on Windows.") + } + + watcher := newWatcher(t) + + testDir := tempMkdir(t) + defer os.RemoveAll(testDir) + + link := path.Join(testDir, "link") + if err := os.Symlink(".", link); err != nil { + t.Fatalf("could not make symlink: %v", err) + } + addWatch(t, watcher, testDir) + + var createEventsReceived counter + go func() { + for ev := range watcher.Events { + if ev.Op&Create == Create { + createEventsReceived.increment() + } + } + }() + + if err := os.Remove(link); err != nil { + t.Fatalf("Error removing link: %v", err) + } + + // It would be nice to be able to expect a delete event here, but kqueue has + // no way for us to get events on symlinks themselves, because opening them + // opens an fd to the file to which they point. + + if err := ioutil.WriteFile(link, []byte("foo"), 0700); err != nil { + t.Fatalf("could not make symlink: %v", err) + } + + // We expect this event to be received almost immediately, but let's wait 500 ms to be sure + time.Sleep(500 * time.Millisecond) + + if got := createEventsReceived.value(); got == 0 { + t.Errorf("want at least 1 create event got %v", got) + } + + watcher.Close() +} + // TestConcurrentRemovalOfWatch tests that concurrent calls to RemoveWatch do not race. // See https://codereview.appspot.com/103300045/ // go test -test.run=TestConcurrentRemovalOfWatch -test.cpu=1,1,1,1,1 -race diff --git a/kqueue.go b/kqueue.go index 821955f..9662a50 100644 --- a/kqueue.go +++ b/kqueue.go @@ -94,7 +94,8 @@ func (w *Watcher) Add(name string) error { w.mu.Lock() w.externalWatches[name] = true w.mu.Unlock() - return w.addWatch(name, noteAllEvents) + _, err := w.addWatch(name, noteAllEvents) + return err } // Remove stops watching the the named file or directory (non-recursively). @@ -153,7 +154,8 @@ var keventWaitTime = durationToTimespec(100 * time.Millisecond) // addWatch adds name to the watched file set. // The flags are interpreted as described in kevent(2). -func (w *Watcher) addWatch(name string, flags uint32) error { +// Returns the real path to the file which was added, if any, which may be different from the one passed in the case of symlinks. +func (w *Watcher) addWatch(name string, flags uint32) (string, error) { var isDir bool // Make ./name and name equivalent name = filepath.Clean(name) @@ -161,7 +163,7 @@ func (w *Watcher) addWatch(name string, flags uint32) error { w.mu.Lock() if w.isClosed { w.mu.Unlock() - return errors.New("kevent instance already closed") + return "", errors.New("kevent instance already closed") } watchfd, alreadyWatching := w.watches[name] // We already have a watch, but we can still override flags. @@ -173,17 +175,17 @@ func (w *Watcher) addWatch(name string, flags uint32) error { if !alreadyWatching { fi, err := os.Lstat(name) if err != nil { - return err + return "", err } // Don't watch sockets. if fi.Mode()&os.ModeSocket == os.ModeSocket { - return nil + return "", nil } // Don't watch named pipes. if fi.Mode()&os.ModeNamedPipe == os.ModeNamedPipe { - return nil + return "", nil } // Follow Symlinks @@ -195,18 +197,26 @@ func (w *Watcher) addWatch(name string, flags uint32) error { if fi.Mode()&os.ModeSymlink == os.ModeSymlink { name, err = filepath.EvalSymlinks(name) if err != nil { - return nil + return "", nil + } + + w.mu.Lock() + _, alreadyWatching = w.watches[name] + w.mu.Unlock() + + if alreadyWatching { + return name, nil } fi, err = os.Lstat(name) if err != nil { - return nil + return "", nil } } watchfd, err = syscall.Open(name, openMode, 0700) if watchfd == -1 { - return err + return "", err } isDir = fi.IsDir() @@ -215,7 +225,7 @@ func (w *Watcher) addWatch(name string, flags uint32) error { const registerAdd = syscall.EV_ADD | syscall.EV_CLEAR | syscall.EV_ENABLE if err := register(w.kq, []int{watchfd}, registerAdd, flags); err != nil { syscall.Close(watchfd) - return err + return "", err } if !alreadyWatching { @@ -229,6 +239,7 @@ func (w *Watcher) addWatch(name string, flags uint32) error { // Watch the directory if it has not been watched before, // or if it was watched before, but perhaps only a NOTE_DELETE (watchDirectoryFiles) w.mu.Lock() + watchDir := (flags&syscall.NOTE_WRITE) == syscall.NOTE_WRITE && (!alreadyWatching || (w.dirFlags[name]&syscall.NOTE_WRITE) != syscall.NOTE_WRITE) // Store flags so this watch can be updated later @@ -237,11 +248,11 @@ func (w *Watcher) addWatch(name string, flags uint32) error { if watchDir { if err := w.watchDirectoryFiles(name); err != nil { - return err + return "", err } } } - return nil + return name, nil } // readEvents reads from kqueue and converts the received kevents into @@ -364,7 +375,8 @@ func (w *Watcher) watchDirectoryFiles(dirPath string) error { for _, fileInfo := range files { filePath := filepath.Join(dirPath, fileInfo.Name()) - if err := w.internalWatch(filePath, fileInfo); err != nil { + filePath, err = w.internalWatch(filePath, fileInfo) + if err != nil { return err } @@ -399,7 +411,8 @@ func (w *Watcher) sendDirectoryChangeEvents(dirPath string) { } // like watchDirectoryFiles (but without doing another ReadDir) - if err := w.internalWatch(filePath, fileInfo); err != nil { + filePath, err = w.internalWatch(filePath, fileInfo) + if err != nil { return } @@ -409,7 +422,7 @@ func (w *Watcher) sendDirectoryChangeEvents(dirPath string) { } } -func (w *Watcher) internalWatch(name string, fileInfo os.FileInfo) error { +func (w *Watcher) internalWatch(name string, fileInfo os.FileInfo) (string, error) { if fileInfo.IsDir() { // mimic Linux providing delete events for subdirectories // but preserve the flags used if currently watching subdirectory -- 2.50.1