inotify: Avoid most of the race conditions, get rid of Cint

There's still a condition that could be a race with fd-reuse, if one
goroutine is calling inotify.{AddWatch,RmWatch,Read}(); another
goroutine is calling inotify.Close(), and several things happen between
loadFd() running and the add_watch/rm_watch/read syscall launching:
 - syscall.Close() returns
 - syscall.Open() reuses the filedescriptor

           A                                  B
    syscall(loadFd())         inotify.Close(); syscall.Open()
    ----------------------------------------------------------
           loadFd()
                                     syscall.Close()
                                     syscall.Open()
           syscall()

Given that Read() can't be allowed to block Close() from running, I'm not
sure there's a way to fix this.

5 files changed, 54 insertions, 58 deletions

diff --git a/src/inotify/bits.go b/src/inotify/bits.go
index 3606b52..9162435 100644
--- a/src/inotify/bits.go
+++ b/src/inotify/bits.go
@@ -1,5 +1,7 @@
 package inotify
 
+import "sync/atomic"
+
 const (
 	// Flags for the parameter of InotifyInit1().
 	// These, oddly, appear to be 24-bit numbers.
@@ -7,8 +9,19 @@ const (
 	IN_NONBLOCK uint32 = 00004000
 )
 
-type Mask uint32
+// Logically, Fd and Wd should be 'int', not 'int64', to match the OS.
+// But, because there's no 'sync/atomic.SwapInt', we cast up to int64.
+type Wd int64
+type Fd int64
+
+func swapFd(addr *Fd, new Fd) (old Fd) {
+	return Fd(atomic.SwapInt64((*int64)(addr), int64(new)))
+}
+func loadFd(addr *Fd) Fd {
+	return Fd(atomic.LoadInt64((*int64)(addr)))
+}
 
+type Mask uint32
 const (
 	// Supported events suitable for the `mask` parameter of Inotify.AddWatch().
 	IN_ACCESS        Mask = (1<< 0) // File was accessed.
diff --git a/src/inotify/inotify.go b/src/inotify/inotify.go
index 6d74830..f7db1b0 100644
--- a/src/inotify/inotify.go
+++ b/src/inotify/inotify.go
@@ -1,23 +1,21 @@
 package inotify
 
 import (
-	"errors"
 	"syscall"
 	"unsafe"
+	"sync"
 )
 
-var InotifyAlreadyClosedError error = errors.New("inotify instance already closed")
-
 type Inotify struct {
-	fd       Cint
-	isClosed bool
+	fd       Fd
 
 	fullbuff [4096]byte
 	buff     []byte
+	buffLock sync.Mutex
 }
 
 type Event struct {
-	Wd     Cint    /* Watch descriptor */
+	Wd     Wd      /* Watch descriptor */
 	Mask   Mask    /* Mask describing event */
 	Cookie uint32  /* Unique cookie associating related events (for rename(2)) */
 	Name   *string /* Optional name */
@@ -26,54 +24,42 @@ type Event struct {
 func InotifyInit() (*Inotify, error) {
 	fd, err := inotify_init()
 	o := Inotify{
-		fd:       Cint(fd),
-		isClosed: false,
+		fd: fd,
 	}
 	o.buff = o.fullbuff[:0]
 	return &o, err
 }
 
-func InotifyInit1(flags Cint) (*Inotify, error) {
+func InotifyInit1(flags int) (*Inotify, error) {
 	fd, err := inotify_init1(flags)
 	o := Inotify{
-		fd:       Cint(fd),
-		isClosed: false,
+		fd: fd,
 	}
 	o.buff = o.fullbuff[:0]
 	return &o, err
 }
 
-func (o *Inotify) AddWatch(path string, mask Mask) (Cint, error) {
-	if o.isClosed {
-		return -1, InotifyAlreadyClosedError
-	}
-	return inotify_add_watch(o.fd, path, uint32(mask))
+func (o *Inotify) AddWatch(path string, mask Mask) (Wd, error) {
+	return inotify_add_watch(loadFd(&o.fd), path, mask)
 }
 
-func (o *Inotify) RmWatch(wd Cint) error {
-	if o.isClosed {
-		return InotifyAlreadyClosedError
-	}
-	return inotify_rm_watch(o.fd, wd)
+func (o *Inotify) RmWatch(wd Wd) error {
+	return inotify_rm_watch(loadFd(&o.fd), wd)
 }
 
 func (o *Inotify) Close() error {
-	if o.isClosed {
-		return InotifyAlreadyClosedError
-	}
-	o.isClosed = true
-	return sysclose(o.fd)
+	return sysclose(swapFd(&o.fd, -1))
 }
 
 func (o *Inotify) Read() (Event, error) {
+	o.buffLock.Lock()
+	defer o.buffLock.Unlock()
+
 	if len(o.buff) == 0 {
-		if o.isClosed {
-			return Event{Wd: -1}, InotifyAlreadyClosedError
-		}
-		len, err := sysread(o.fd, o.buff)
+		len, err := sysread(loadFd(&o.fd), o.buff)
 		if len == 0 {
 			return Event{Wd: -1}, o.Close()
-		} else if len <= 0 {
+		} else if len < 0 {
 			return Event{Wd: -1}, err
 		}
 		o.buff = o.fullbuff[0:len]
@@ -81,7 +67,7 @@ func (o *Inotify) Read() (Event, error) {
 
 	raw := (*syscall.InotifyEvent)(unsafe.Pointer(&o.buff[0]))
 	ret := Event{
-		Wd:     Cint(raw.Wd),
+		Wd:     Wd(raw.Wd),
 		Mask:   Mask(raw.Mask),
 		Cookie: raw.Cookie,
 		Name:   nil,
diff --git a/src/inotify/syscall.go b/src/inotify/syscall.go
index 1b5c426..721a10a 100644
--- a/src/inotify/syscall.go
+++ b/src/inotify/syscall.go
@@ -1,14 +1,11 @@
 package inotify
 
 import (
-	"C"
 	"os"
 	"syscall"
 )
 
-type Cint C.int
-
-func pathError(op string, path string, err error) error {
+func newPathError(op string, path string, err error) error {
 	if err == nil {
 		return nil
 	}
@@ -16,26 +13,26 @@ func pathError(op string, path string, err error) error {
 }
 
 /* Create and initialize inotify instance.  */
-func inotify_init() (Cint, error) {
+func inotify_init() (Fd, error) {
 	fd, errno := syscall.InotifyInit()
-	return Cint(fd), os.NewSyscallError("inotify_init", errno)
+	return Fd(fd), os.NewSyscallError("inotify_init", errno)
 }
 
 /* Create and initialize inotify instance.  */
-func inotify_init1(flags Cint) (Cint, error) {
-	fd, errno := syscall.InotifyInit1(int(flags))
-	return Cint(fd), os.NewSyscallError("inotify_init1", errno)
+func inotify_init1(flags int) (Fd, error) {
+	fd, errno := syscall.InotifyInit1(flags)
+	return Fd(fd), os.NewSyscallError("inotify_init1", errno)
 }
 
 /* Add watch of object NAME to inotify instance FD.  Notify about
    events specified by MASK.  */
-func inotify_add_watch(fd Cint, name string, mask uint32) (Cint, error) {
-	wd, errno := syscall.InotifyAddWatch(int(fd), name, mask)
-	return Cint(wd), pathError("inotify_add_watch", name, errno)
+func inotify_add_watch(fd Fd, name string, mask Mask) (Wd, error) {
+	wd, errno := syscall.InotifyAddWatch(int(fd), name, uint32(mask))
+	return Wd(wd), newPathError("inotify_add_watch", name, errno)
 }
 
 /* Remove the watch specified by WD from the inotify instance FD.  */
-func inotify_rm_watch(fd Cint, wd Cint) error {
+func inotify_rm_watch(fd Fd, wd Wd) error {
 	success, errno := syscall.InotifyRmWatch(int(fd), uint32(wd))
 	switch success {
 	case -1:
@@ -52,11 +49,11 @@ func inotify_rm_watch(fd Cint, wd Cint) error {
 	panic("should never happen")
 }
 
-func sysclose(fd Cint) error {
+func sysclose(fd Fd) error {
 	return os.NewSyscallError("close", syscall.Close(int(fd)))
 }
 
-func sysread(fd Cint, p []byte) (Cint, error) {
+func sysread(fd Fd, p []byte) (int, error) {
 	n, err := syscall.Read(int(fd), p)
-	return Cint(n), os.NewSyscallError("read", err)
+	return n, os.NewSyscallError("read", err)
 }
diff --git a/src/nshd/hackers_git/hackers.go b/src/nshd/hackers_git/hackers.go
index 0f2dd3d..ca3974d 100644
--- a/src/nshd/hackers_git/hackers.go
+++ b/src/nshd/hackers_git/hackers.go
@@ -28,10 +28,10 @@ type Hackers struct {
 	users     map[int32]user
 
 	in_fd      *inotify.Inotify
-	in_wd_home inotify.Cint
-	in_wd_yaml inotify.Cint
-	in_uid2wd  map[int32]inotify.Cint
-	in_wd2uid  map[inotify.Cint]int32
+	in_wd_home inotify.Wd
+	in_wd_yaml inotify.Wd
+	in_uid2wd  map[int32]inotify.Wd
+	in_wd2uid  map[inotify.Wd]int32
 }
 
 var _ nslcd_systemd.Backend = &Hackers{}
diff --git a/src/nshd/hackers_git/hackers_watch.go b/src/nshd/hackers_git/hackers_watch.go
index da008f8..b273741 100644
--- a/src/nshd/hackers_git/hackers_watch.go
+++ b/src/nshd/hackers_git/hackers_watch.go
@@ -36,7 +36,7 @@ func (o *Hackers) watchHomedir(uid int32) {
 	o.users[uid] = user
 }
 
-func (o *Hackers) unwatchHomedir(wd inotify.Cint) {
+func (o *Hackers) unwatchHomedir(wd inotify.Wd) {
 	err := o.in_fd.RmWatch(wd)
 	if err != nil {
 		logger.Warning("hackers.git: %v", err)
@@ -57,8 +57,8 @@ func (o *Hackers) close() {
 	o.in_wd_home = -1
 	o.in_wd_yaml = -1
 	o.users = make(map[int32]user, 0)
-	o.in_uid2wd = make(map[int32]inotify.Cint, 0)
-	o.in_wd2uid = make(map[inotify.Cint]int32, 0)
+	o.in_uid2wd = make(map[int32]inotify.Wd, 0)
+	o.in_wd2uid = make(map[inotify.Wd]int32, 0)
 }
 
 func (o *Hackers) reload() (err error) {
@@ -69,8 +69,8 @@ func (o *Hackers) reload() (err error) {
 
 	filenames, err := filepath.Glob(o.cfg.Yamldir + "/*.yml")
 	o.users = make(map[int32]user, len(filenames))
-	o.in_uid2wd = make(map[int32]inotify.Cint, len(filenames))
-	o.in_wd2uid = make(map[inotify.Cint]int32, len(filenames))
+	o.in_uid2wd = make(map[int32]inotify.Wd, len(filenames))
+	o.in_wd2uid = make(map[inotify.Wd]int32, len(filenames))
 	for _, filename := range filenames {
 		logger.Debug("hackers.git: Loading yaml file: %s", filename)
 		user, err := load_user_yaml(filename)