aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJakob Unterwurzacher2016-01-25 00:51:28 +0100
committerJakob Unterwurzacher2016-01-25 00:51:28 +0100
commit4c9e249e3ac1fc2995e01eb1ed24799c3a3bc66b (patch)
tree97a9eb498ef70a3c00dd7bd8fafb4765abeecf5b
parent4259c8f7ebc9f5d167556ed142e82e8232706de6 (diff)
Convert fdLock to an RWMutex and protect the whole transaction
...against concurrent closes. The testcase (set -e; while true; do truncate -s $RANDOM b; done) & (set -e; while true; do truncate -s $RANDOM b; done) & uncovered lots of unnecessary RMW failures that were the result of concurrent closes. With this patch, the only remaining error is "Truncate on forgotten file" that is probably caused by a problem in the go-fuse lib ( https://github.com/hanwen/go-fuse/issues/95 )
-rw-r--r--cryptfs/names_diriv.go2
-rw-r--r--pathfs_frontend/file.go106
2 files changed, 58 insertions, 50 deletions
diff --git a/cryptfs/names_diriv.go b/cryptfs/names_diriv.go
index 9519ebc..45abfce 100644
--- a/cryptfs/names_diriv.go
+++ b/cryptfs/names_diriv.go
@@ -1,9 +1,9 @@
package cryptfs
import (
- "os"
"fmt"
"io/ioutil"
+ "os"
"path/filepath"
"strings"
"sync"
diff --git a/pathfs_frontend/file.go b/pathfs_frontend/file.go
index ceb9216..33ad0c7 100644
--- a/pathfs_frontend/file.go
+++ b/pathfs_frontend/file.go
@@ -20,12 +20,11 @@ import (
type file struct {
fd *os.File
- // os.File is not threadsafe. Although fd themselves are
- // constant during the lifetime of an open file, the OS may
- // reuse the fd number after it is closed. When open races
- // with another close, they may lead to confusion as which
- // file gets written in the end.
- fdLock sync.Mutex
+ // fdLock prevents the fd to be closed while we are in the middle of
+ // an operation.
+ // Every FUSE entrypoint should RLock(). The only user of Lock() is
+ // Release(), which closes the fd, hence has to acquire an exclusive lock.
+ fdLock sync.RWMutex
// Was the file opened O_WRONLY?
writeOnly bool
@@ -38,6 +37,8 @@ type file struct {
// File header
header *cryptfs.FileHeader
+
+ forgotten bool
}
func NewFile(fd *os.File, writeOnly bool, cfs *cryptfs.CryptFS) nodefs.File {
@@ -53,6 +54,12 @@ func NewFile(fd *os.File, writeOnly bool, cfs *cryptfs.CryptFS) nodefs.File {
}
}
+// intFd - return the backing file descriptor as an integer. Used for debug
+// messages.
+func (f *file) intFd() int {
+ return int(f.fd.Fd())
+}
+
func (f *file) InnerFile() nodefs.File {
return nil
}
@@ -84,11 +91,9 @@ func (f *file) createHeader() error {
buf := h.Pack()
// Prevent partially written (=corrupt) header by preallocating the space beforehand
- f.fdLock.Lock()
- defer f.fdLock.Unlock()
err := prealloc(int(f.fd.Fd()), 0, cryptfs.HEADER_LEN)
if err != nil {
- cryptfs.Warn.Printf("createHeader: fallocateRetry failed: %s\n", err.Error())
+ cryptfs.Warn.Printf("ino%d: createHeader: prealloc failed: %s\n", f.ino, err.Error())
return err
}
@@ -133,9 +138,7 @@ func (f *file) doRead(off uint64, length uint64) ([]byte, fuse.Status) {
skip := blocks[0].Skip
cryptfs.Debug.Printf("JointCiphertextRange(%d, %d) -> %d, %d, %d", off, length, alignedOffset, alignedLength, skip)
ciphertext := make([]byte, int(alignedLength))
- f.fdLock.Lock()
n, err := f.fd.ReadAt(ciphertext, int64(alignedOffset))
- f.fdLock.Unlock()
if err != nil && err != io.EOF {
cryptfs.Warn.Printf("read: ReadAt: %s", err.Error())
return nil, fuse.ToStatus(err)
@@ -173,6 +176,8 @@ func (f *file) doRead(off uint64, length uint64) ([]byte, fuse.Status) {
// Read - FUSE call
func (f *file) Read(buf []byte, off int64) (resultData fuse.ReadResult, code fuse.Status) {
+ f.fdLock.RLock()
+ defer f.fdLock.RUnlock()
cryptfs.Debug.Printf("ino%d: FUSE Read: offset=%d length=%d", f.ino, len(buf), off)
@@ -231,7 +236,7 @@ func (f *file) doWrite(data []byte, off int64) (uint32, fuse.Status) {
o, _ := b.PlaintextRange()
oldData, status := f.doRead(o, f.cfs.PlainBS())
if status != fuse.OK {
- cryptfs.Warn.Printf("RMW read failed: %s", status.String())
+ cryptfs.Warn.Printf("ino%d fh%d: RMW read failed: %s", f.ino, f.intFd(), status.String())
return written, status
}
// Modify
@@ -246,19 +251,15 @@ func (f *file) doWrite(data []byte, off int64) (uint32, fuse.Status) {
f.ino, uint64(len(blockData))-f.cfs.BlockOverhead(), b.BlockNo)
// Prevent partially written (=corrupt) blocks by preallocating the space beforehand
- f.fdLock.Lock()
err := prealloc(int(f.fd.Fd()), int64(blockOffset), int64(blockLen))
- f.fdLock.Unlock()
if err != nil {
- cryptfs.Warn.Printf("doWrite: fallocateRetry failed: %s", err.Error())
+ cryptfs.Warn.Printf("ino%d fh%d: doWrite: prealloc failed: %s", f.ino, f.intFd(), err.Error())
status = fuse.ToStatus(err)
break
}
// Write
- f.fdLock.Lock()
_, err = f.fd.WriteAt(blockData, int64(blockOffset))
- f.fdLock.Unlock()
if err != nil {
cryptfs.Warn.Printf("doWrite: Write failed: %s", err.Error())
@@ -272,10 +273,13 @@ func (f *file) doWrite(data []byte, off int64) (uint32, fuse.Status) {
// Write - FUSE call
func (f *file) Write(data []byte, off int64) (uint32, fuse.Status) {
- cryptfs.Debug.Printf("ino%d: FUSE Write: offset=%d length=%d", f.ino, off, len(data))
+ f.fdLock.RLock()
+ defer f.fdLock.RUnlock()
wlock.lock(f.ino)
defer wlock.unlock(f.ino)
+ cryptfs.Debug.Printf("ino%d: FUSE Write: offset=%d length=%d", f.ino, off, len(data))
+
fi, err := f.fd.Stat()
if err != nil {
cryptfs.Warn.Printf("Write: Fstat failed: %v", err)
@@ -292,23 +296,24 @@ func (f *file) Write(data []byte, off int64) (uint32, fuse.Status) {
return f.doWrite(data, off)
}
-// Release - FUSE call, forget file
+// Release - FUSE call, close file
func (f *file) Release() {
f.fdLock.Lock()
+ defer f.fdLock.Unlock()
+
f.fd.Close()
- f.fdLock.Unlock()
- wlock.unregister(f.ino)
+ f.forgotten = true
}
// Flush - FUSE call
func (f *file) Flush() fuse.Status {
- f.fdLock.Lock()
+ f.fdLock.RLock()
+ defer f.fdLock.RUnlock()
// Since Flush() may be called for each dup'd fd, we don't
// want to really close the file, we just want to flush. This
// is achieved by closing a dup'd fd.
newFd, err := syscall.Dup(int(f.fd.Fd()))
- f.fdLock.Unlock()
if err != nil {
return fuse.ToStatus(err)
@@ -318,25 +323,28 @@ func (f *file) Flush() fuse.Status {
}
func (f *file) Fsync(flags int) (code fuse.Status) {
- f.fdLock.Lock()
- r := fuse.ToStatus(syscall.Fsync(int(f.fd.Fd())))
- f.fdLock.Unlock()
+ f.fdLock.RLock()
+ defer f.fdLock.RUnlock()
- return r
+ return fuse.ToStatus(syscall.Fsync(int(f.fd.Fd())))
}
// Truncate - FUSE call
func (f *file) Truncate(newSize uint64) fuse.Status {
+ f.fdLock.RLock()
+ defer f.fdLock.RUnlock()
wlock.lock(f.ino)
defer wlock.unlock(f.ino)
+ if f.forgotten {
+ cryptfs.Warn.Printf("ino%d fh%d: Truncate on forgotten file", f.ino, f.intFd())
+ }
+
// Common case first: Truncate to zero
if newSize == 0 {
- f.fdLock.Lock()
err := syscall.Ftruncate(int(f.fd.Fd()), 0)
- f.fdLock.Unlock()
if err != nil {
- cryptfs.Warn.Printf("ino%d: Ftruncate(fd, 0) returned error: %v", f.ino, err)
+ cryptfs.Warn.Printf("ino%d fh%d: Ftruncate(fd, 0) returned error: %v", f.ino, f.intFd(), err)
return fuse.ToStatus(err)
}
// Truncate to zero kills the file header
@@ -348,7 +356,7 @@ func (f *file) Truncate(newSize uint64) fuse.Status {
// the file
fi, err := f.fd.Stat()
if err != nil {
- cryptfs.Warn.Printf("ino%d: Truncate: Fstat failed: %v", f.ino, err)
+ cryptfs.Warn.Printf("ino%d fh%d: Truncate: Fstat failed: %v", f.ino, f.intFd(), err)
return fuse.ToStatus(err)
}
oldSize := f.cfs.CipherSizeToPlainSize(uint64(fi.Size()))
@@ -358,6 +366,11 @@ func (f *file) Truncate(newSize uint64) fuse.Status {
cryptfs.Debug.Printf("ino%d: FUSE Truncate from %.2f to %.2f blocks (%d to %d bytes)", f.ino, oldB, newB, oldSize, newSize)
}
+ // File size stays the same - nothing to do
+ if newSize == oldSize {
+ return fuse.OK
+ }
+
// File grows
if newSize > oldSize {
@@ -381,9 +394,7 @@ func (f *file) Truncate(newSize uint64) fuse.Status {
}
} else {
off, length := b.CiphertextRange()
- f.fdLock.Lock()
err := syscall.Ftruncate(int(f.fd.Fd()), int64(off+length))
- f.fdLock.Unlock()
if err != nil {
cryptfs.Warn.Printf("grow Ftruncate returned error: %v", err)
return fuse.ToStatus(err)
@@ -407,9 +418,7 @@ func (f *file) Truncate(newSize uint64) fuse.Status {
}
}
// Truncate down to last complete block
- f.fdLock.Lock()
err = syscall.Ftruncate(int(f.fd.Fd()), int64(cipherOff))
- f.fdLock.Unlock()
if err != nil {
cryptfs.Warn.Printf("shrink Ftruncate returned error: %v", err)
return fuse.ToStatus(err)
@@ -424,27 +433,26 @@ func (f *file) Truncate(newSize uint64) fuse.Status {
}
func (f *file) Chmod(mode uint32) fuse.Status {
- f.fdLock.Lock()
- r := fuse.ToStatus(f.fd.Chmod(os.FileMode(mode)))
- f.fdLock.Unlock()
+ f.fdLock.RLock()
+ defer f.fdLock.RUnlock()
- return r
+ return fuse.ToStatus(f.fd.Chmod(os.FileMode(mode)))
}
func (f *file) Chown(uid uint32, gid uint32) fuse.Status {
- f.fdLock.Lock()
- r := fuse.ToStatus(f.fd.Chown(int(uid), int(gid)))
- f.fdLock.Unlock()
+ f.fdLock.RLock()
+ defer f.fdLock.RUnlock()
- return r
+ return fuse.ToStatus(f.fd.Chown(int(uid), int(gid)))
}
func (f *file) GetAttr(a *fuse.Attr) fuse.Status {
+ f.fdLock.RLock()
+ defer f.fdLock.RUnlock()
+
cryptfs.Debug.Printf("file.GetAttr()")
st := syscall.Stat_t{}
- f.fdLock.Lock()
err := syscall.Fstat(int(f.fd.Fd()), &st)
- f.fdLock.Unlock()
if err != nil {
return fuse.ToStatus(err)
}
@@ -469,6 +477,9 @@ func (f *file) Allocate(off uint64, sz uint64, mode uint32) fuse.Status {
const _UTIME_OMIT = ((1 << 30) - 2)
func (f *file) Utimens(a *time.Time, m *time.Time) fuse.Status {
+ f.fdLock.RLock()
+ defer f.fdLock.RUnlock()
+
ts := make([]syscall.Timespec, 2)
if a == nil {
@@ -483,9 +494,6 @@ func (f *file) Utimens(a *time.Time, m *time.Time) fuse.Status {
ts[1].Sec = m.Unix()
}
- f.fdLock.Lock()
fn := fmt.Sprintf("/proc/self/fd/%d", f.fd.Fd())
- err := syscall.UtimesNano(fn, ts)
- f.fdLock.Unlock()
- return fuse.ToStatus(err)
+ return fuse.ToStatus(syscall.UtimesNano(fn, ts))
}