Thread safety

README.md

@@ -3,4 +3,4 @@ libgocryptfs is a re-desing of the original [gocryptfs](https://github.com/rfjak
 - Reduce attack surface by restricting volumes access to only one process rather than one user
 
 ## Warning !
-The only goal of this library is to be integrated in [DroidFS](https://forge.chapril.org/hardcoresushi/DroidFS). It's not actually ready for other usages. libgocryptfs doesn't implement all features provided by gocryptfs like symbolic links creation, thread-safety, reverse volume creation... Use it at your own risk !
+The only goal of this library is to be integrated in [DroidFS](https://forge.chapril.org/hardcoresushi/DroidFS). It's not actually ready for other usages. libgocryptfs doesn't implement all features provided by gocryptfs like symbolic links, editing attributes, creating reverse volume... Use it at your own risk !

common_ops.go

@@ -11,10 +11,11 @@ import (
 
 //export gcf_get_attrs
 func gcf_get_attrs(sessionID int, relPath string) (uint64, int64, bool) {
-	volume, ok := OpenedVolumes[sessionID]
+	value, ok := OpenedVolumes.Load(sessionID)
 	if !ok {
 		return 0, 0, false
 	}
+	volume := value.(*Volume)
 	dirfd, cName, err := volume.prepareAtSyscall(relPath)
 	if err != nil {
 		return 0, 0, false
@@ -35,10 +36,11 @@ func gcf_get_attrs(sessionID int, relPath string) (uint64, int64, bool) {
 // libgocryptfs: using Renameat instead of Renameat2 to support older kernels
 //export gcf_rename
 func gcf_rename(sessionID int, oldPath string, newPath string) bool {
-	volume, ok := OpenedVolumes[sessionID]
+	value, ok := OpenedVolumes.Load(sessionID)
 	if !ok {
 		return false
 	}
+	volume := value.(*Volume)
 	dirfd, cName, err := volume.prepareAtSyscall(oldPath)
 	if err != nil {
 		return false

dircache.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"log"
+	"sync"
 	"syscall"
 	"time"
 )
@@ -40,6 +41,7 @@ func (e *dirCacheEntry) Clear() {
 }
 
 type dirCache struct {
+	sync.Mutex
 	// Expected length of the stored IVs. Only used for sanity checks.
 	// Usually set to 16, but 0 in plaintextnames mode.
 	ivLen int
@@ -57,6 +59,8 @@ type dirCache struct {
 
 // Clear clears the cache contents.
 func (d *dirCache) Clear() {
+	d.Lock()
+	defer d.Unlock()
 	for i := range d.entries {
 		d.entries[i].Clear()
 	}
@@ -70,6 +74,8 @@ func (d *dirCache) Store(path string, fd int, iv []byte) {
 	if fd <= 0 || len(iv) != d.ivLen {
 		log.Panicf("Store sanity check failed: fd=%d len=%d", fd, len(iv))
 	}
+	d.Lock()
+	defer d.Unlock()
 	e := &d.entries[d.nextIndex]
 	// Round-robin works well enough
 	d.nextIndex = (d.nextIndex + 1) % dirCacheSize
@@ -93,6 +99,8 @@ func (d *dirCache) Store(path string, fd int, iv []byte) {
 // It returns (-1, nil) if not found. The fd is internally Dup()ed and the
 // caller must close it when done.
 func (d *dirCache) Lookup(path string) (fd int, iv []byte) {
+	d.Lock()
+	defer d.Unlock()
 	if enableStats {
 		d.lookups++
 	}
@@ -134,3 +142,13 @@ func (d *dirCache) expireThread() {
 		d.Clear()
 	}
 }
+
+func (d* dirCache) Delete(path string) {
+	for i := range d.entries {
+		e := &d.entries[i]
+		if e.path == path {
+			e.Clear()
+			break
+		}
+	}
+}

directory.go

@@ -17,10 +17,12 @@ import (
 	"libgocryptfs/v2/internal/syscallcompat"
 )
 
-func mkdirWithIv(dirfd int, cName string, mode uint32) error {
+func (volume *Volume) mkdirWithIv(dirfd int, cName string, mode uint32) error {
 	// Between the creation of the directory and the creation of gocryptfs.diriv
 	// the directory is inconsistent. Take the lock to prevent other readers
 	// from seeing it.
+	volume.dirIVLock.Lock()
+	defer volume.dirIVLock.Unlock()
 	err := unix.Mkdirat(dirfd, cName, mode)
 	if err != nil {
 		return err
@@ -40,10 +42,11 @@ func mkdirWithIv(dirfd int, cName string, mode uint32) error {
 
 //export gcf_list_dir
 func gcf_list_dir(sessionID int, dirName string) (*C.char, *C.int, C.int) {
-	volume, ok := OpenedVolumes[sessionID]
+	value, ok := OpenedVolumes.Load(sessionID)
 	if !ok {
 		return nil, nil, 0
 	}
+	volume := value.(*Volume)
 	parentDirFd, cDirName, err := volume.prepareAtSyscallMyself(dirName)
 	if err != nil {
 		return nil, nil, 0
@@ -119,10 +122,11 @@ func gcf_list_dir(sessionID int, dirName string) (*C.char, *C.int, C.int) {
 
 //export gcf_mkdir
 func gcf_mkdir(sessionID int, path string, mode uint32) bool {
-	volume, ok := OpenedVolumes[sessionID]
+	value, ok := OpenedVolumes.Load(sessionID)
 	if !ok {
 		return false
 	}
+	volume := value.(*Volume)
 	dirfd, cName, err := volume.prepareAtSyscall(path)
 	if err != nil {
 		return false
@@ -155,13 +159,13 @@ func gcf_mkdir(sessionID int, path string, mode uint32) bool {
 			}
 
 			// Create directory
-			err = mkdirWithIv(dirfd, cName, mode)
+			err = volume.mkdirWithIv(dirfd, cName, mode)
 			if err != nil {
 				nametransform.DeleteLongNameAt(dirfd, cName)
 				return false
 			}
 		} else {
-			err = mkdirWithIv(dirfd, cName, mode)
+			err = volume.mkdirWithIv(dirfd, cName, mode)
 			if err != nil {
 				return false
 			}
@@ -193,10 +197,11 @@ func gcf_mkdir(sessionID int, path string, mode uint32) bool {
 
 //export gcf_rmdir
 func gcf_rmdir(sessionID int, relPath string) bool {
-	volume, ok := OpenedVolumes[sessionID]
+	value, ok := OpenedVolumes.Load(sessionID)
 	if !ok {
 		return false
 	}
+	volume := value.(*Volume)
 	parentDirFd, cName, err := volume.prepareAtSyscall(relPath)
 	if err != nil {
 		return false
@@ -229,6 +234,10 @@ func gcf_rmdir(sessionID int, relPath string) bool {
 	}
 	// Move "gocryptfs.diriv" to the parent dir as "gocryptfs.diriv.rmdir.XYZ"
 	tmpName := fmt.Sprintf("%s.rmdir.%d", nametransform.DirIVFilename, cryptocore.RandUint64())
+	// The directory is in an inconsistent state between rename and rmdir.
+	// Protect against concurrent readers.
+	volume.dirIVLock.Lock()
+	defer volume.dirIVLock.Unlock()
 	err = syscallcompat.Renameat(dirfd, nametransform.DirIVFilename, parentDirFd, tmpName)
 	if err != nil {
 		return false
@@ -247,5 +256,6 @@ func gcf_rmdir(sessionID int, relPath string) bool {
 	if nametransform.IsLongContent(cName) {
 		nametransform.DeleteLongNameAt(parentDirFd, cName)
 	}
+	volume.dirCache.Delete(relPath)
 	return true
 }

file.go

@@ -41,13 +41,16 @@ func (volume *Volume) registerFileHandle(fd int, cName, path string) int {
 	handleID := -1
 	c := 0
 	for handleID == -1 {
-		_, ok := volume.file_handles[c]
+		_, ok := volume.file_handles.Load(c)
 		if !ok {
 			handleID = c
 		}
 		c++
 	}
-	volume.file_handles[handleID] = File{os.NewFile(uintptr(fd), cName), string([]byte(path[:]))}
+	volume.file_handles.Store(handleID, &File {
+		fd: os.NewFile(uintptr(fd), cName),
+		path: string([]byte(path[:])),
+	})
 	return handleID
 }
 
@@ -100,16 +103,17 @@ func createHeader(fd *os.File) (fileID []byte, err error) {
 // Called by Read() for normal reading,
 // by Write() and Truncate() via doWrite() for Read-Modify-Write.
 func (volume *Volume) doRead(handleID int, dst []byte, off uint64, length uint64) ([]byte, bool) {
-	f, ok := volume.file_handles[handleID]
+	value, ok := volume.file_handles.Load(handleID)
 	if !ok {
 		return nil, false
 	}
+	f := value.(*File)
 	fd := f.fd
 	// Get the file ID, either from the open file table, or from disk.
 	var fileID []byte
-	if volume.fileIDs[handleID] != nil {
+	if f.ID != nil {
 		// Use the cached value in the file table
-		fileID = volume.fileIDs[handleID]
+		fileID = f.ID
 	} else {
 		// Not cached, we have to read it from disk.
 		var err error
@@ -118,9 +122,8 @@ func (volume *Volume) doRead(handleID int, dst []byte, off uint64, length uint64
 			return nil, false
 		}
 		// Save into the file table
-		volume.fileIDs[handleID] = fileID
+		f.ID = fileID
 	}
-
 	// Read the backing ciphertext in one go
 	blocks := volume.contentEnc.ExplodePlainRange(off, length)
 	alignedOffset, alignedLength := blocks[0].JointCiphertextRange(blocks)
@@ -180,19 +183,16 @@ func (volume *Volume) doRead(handleID int, dst []byte, off uint64, length uint64
 //
 // Empty writes do nothing and are allowed.
 func (volume *Volume) doWrite(handleID int, data []byte, off uint64) (uint32, bool) {
-	fileWasEmpty := false
-	// Get the file ID, create a new one if it does not exist yet.
-	var fileID []byte
-
-	f, ok := volume.file_handles[handleID]
+	value, ok := volume.file_handles.Load(handleID)
 	if !ok {
 		return 0, false
 	}
+	f := value.(*File)
 	fd := f.fd
-	// The caller has exclusively locked ContentLock, which blocks all other
-	// readers and writers. No need to take IDLock.
-	if volume.fileIDs[handleID] != nil {
-		fileID = volume.fileIDs[handleID]
+	fileWasEmpty := false
+	var fileID []byte
+	if f.ID != nil {
+		fileID = f.ID
 	} else {
 		// If the file ID is not cached, read it from disk
 		var err error
@@ -205,7 +205,7 @@ func (volume *Volume) doWrite(handleID int, data []byte, off uint64) (uint32, bo
 		if err != nil {
 			return 0, false
 		}
-		volume.fileIDs[handleID] = fileID
+		f.ID = fileID
 	}
 	// Handle payload data
 	dataBuf := bytes.NewBuffer(data)
@@ -299,16 +299,21 @@ func (volume *Volume) truncateGrowFile(handleID int, oldPlainSz uint64, newPlain
 	// The new size is block-aligned. In this case we can do everything ourselves
 	// and avoid the call to doWrite.
 	if newPlainSz%volume.contentEnc.PlainBS() == 0 {
+		value, ok := volume.file_handles.Load(handleID)
+		if !ok {
+			return false
+		}
+		f := value.(*File)
 		// The file was empty, so it did not have a header. Create one.
 		if oldPlainSz == 0 {
-			id, err := createHeader(volume.file_handles[handleID].fd)
+			id, err := createHeader(f.fd)
 			if err != nil {
 				return false
 			}
-			volume.fileIDs[handleID] = id
+			f.ID = id
 		}
 		cSz := int64(volume.contentEnc.PlainSizeToCipherSize(newPlainSz))
-		err := syscall.Ftruncate(int(volume.file_handles[handleID].fd.Fd()), cSz)
+		err := syscall.Ftruncate(int(f.fd.Fd()), cSz)
 		return errToBool(err)
 	}
 	// The new size is NOT aligned, so we need to write a partial block.
@@ -319,7 +324,12 @@ func (volume *Volume) truncateGrowFile(handleID int, oldPlainSz uint64, newPlain
 }
 
 func (volume *Volume) truncate(handleID int, newSize uint64) bool {
-	fileFD := int(volume.file_handles[handleID].fd.Fd())
+	value, ok := volume.file_handles.Load(handleID)
+	if !ok {
+		return false
+	}
+	f := value.(*File)
+	fileFD := int(f.fd.Fd())
 	var err error
 	// Common case first: Truncate to zero
 	if newSize == 0 {
@@ -328,7 +338,7 @@ func (volume *Volume) truncate(handleID int, newSize uint64) bool {
 	}
 	// We need the old file size to determine if we are growing or shrinking
 	// the file
-	oldSize, _, success := gcf_get_attrs(volume.volumeID, volume.file_handles[handleID].path)
+	oldSize, _, success := gcf_get_attrs(volume.volumeID, f.path)
 	if !success {
 		return false
 	}
@@ -369,10 +379,11 @@ func (volume *Volume) truncate(handleID int, newSize uint64) bool {
 
 //export gcf_open_read_mode
 func gcf_open_read_mode(sessionID int, path string) int {
-	volume, ok := OpenedVolumes[sessionID]
+	value, ok := OpenedVolumes.Load(sessionID)
 	if !ok {
 		return -1
 	}
+	volume := value.(*Volume)
 	dirfd, cName, err := volume.prepareAtSyscallMyself(path)
 	if err != nil {
 		return -1
@@ -389,10 +400,11 @@ func gcf_open_read_mode(sessionID int, path string) int {
 
 //export gcf_open_write_mode
 func gcf_open_write_mode(sessionID int, path string, mode uint32) int {
-	volume, ok := OpenedVolumes[sessionID]
+	value, ok := OpenedVolumes.Load(sessionID)
 	if !ok {
 		return -1
 	}
+	volume := value.(*Volume)
 	dirfd, cName, err := volume.prepareAtSyscall(path)
 	if err != nil {
 		return -1
@@ -425,10 +437,11 @@ func gcf_open_write_mode(sessionID int, path string, mode uint32) int {
 
 //export gcf_truncate
 func gcf_truncate(sessionID int, handleID int, offset uint64) bool {
-	volume, ok := OpenedVolumes[sessionID]
+	value, ok := OpenedVolumes.Load(sessionID)
 	if !ok {
 		return false
 	}
+	volume := value.(*Volume)
 	return volume.truncate(handleID, offset)
 }
 
@@ -440,10 +453,21 @@ func gcf_read_file(sessionID, handleID int, offset uint64, dst_buff []byte) uint
 		return 0
 	}
 
-	volume, ok := OpenedVolumes[sessionID]
+	value, ok := OpenedVolumes.Load(sessionID)
+	if !ok {
+		return 0
+	}
+	volume := value.(*Volume)
+
+	value, ok = volume.file_handles.Load(handleID)
 	if !ok {
 		return 0
 	}
+	f := value.(*File)
+	f.fdLock.RLock()
+	defer f.fdLock.RUnlock()
+	f.contentLock.RLock()
+	defer f.contentLock.RUnlock()
 	out, success := volume.doRead(handleID, dst_buff[:0], offset, uint64(length))
 	if !success {
 		return 0
@@ -460,37 +484,49 @@ func gcf_write_file(sessionID, handleID int, offset uint64, data []byte) uint32
 		return 0
 	}
 
-	volume, ok := OpenedVolumes[sessionID]
+	value, ok := OpenedVolumes.Load(sessionID)
 	if !ok {
 		return 0
 	}
+	volume := value.(*Volume)
+
+	value, ok = volume.file_handles.Load(handleID)
+	if !ok {
+		return 0
+	}
+	f := value.(*File)
+	f.fdLock.RLock()
+        defer f.fdLock.RUnlock()
+	f.contentLock.Lock()
+        defer f.contentLock.Unlock()
 	n, _ := volume.doWrite(handleID, data, offset)
 	return n
 }
 
 //export gcf_close_file
 func gcf_close_file(sessionID, handleID int) {
-	volume, ok := OpenedVolumes[sessionID]
+	value, ok := OpenedVolumes.Load(sessionID)
 	if !ok {
 		return
 	}
-	f, ok := volume.file_handles[handleID]
+	volume := value.(*Volume)
+	value, ok = volume.file_handles.Load(handleID)
 	if ok {
+		f := value.(*File)
+		f.fdLock.Lock()
 		f.fd.Close()
-		delete(volume.file_handles, handleID)
-		_, ok := volume.fileIDs[handleID]
-		if ok {
-			delete(volume.fileIDs, handleID)
-		}
+		volume.file_handles.Delete(handleID)
+		f.fdLock.Unlock()
 	}
 }
 
 //export gcf_remove_file
 func gcf_remove_file(sessionID int, path string) bool {
-	volume, ok := OpenedVolumes[sessionID]
+	value, ok := OpenedVolumes.Load(sessionID)
 	if !ok {
 		return false
 	}
+	volume := value.(*Volume)
 	dirfd, cName, err := volume.prepareAtSyscall(path)
 	if err != nil {
 		return false

volume.go

@@ -6,8 +6,9 @@ package main
 import (
 	"C"
 	"os"
-	"path/filepath"
+	"sync"
 	"syscall"
+	"path/filepath"
 
 	"libgocryptfs/v2/internal/configfile"
 	"libgocryptfs/v2/internal/contentenc"
@@ -18,23 +19,35 @@ import (
 )
 
 type File struct {
-	fd   *os.File
-	path string
+	fd          *os.File
+	path        string
+	ID          []byte
+	// 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 and sets "released" to true.
+	fdLock      sync.RWMutex
+	// ContentLock protects on-disk content from concurrent writes. Every writer
+	// must take this lock before modifying the file content.
+	contentLock sync.RWMutex
 }
 
 type Volume struct {
 	volumeID       int
 	rootCipherDir  string
 	plainTextNames bool
+	// dirIVLock: Lock()ed if any "gocryptfs.diriv" file is modified
+	// Readers must RLock() it to prevent them from seeing intermediate
+	// states
+	dirIVLock      sync.RWMutex
 	nameTransform  *nametransform.NameTransform
 	cryptoCore     *cryptocore.CryptoCore
 	contentEnc     *contentenc.ContentEnc
 	dirCache       dirCache
-	file_handles   map[int]File
-	fileIDs        map[int][]byte
+	file_handles   sync.Map
 }
 
-var OpenedVolumes map[int]*Volume
+var OpenedVolumes sync.Map
 
 func wipe(d []byte) {
 	for i := range d {
@@ -79,25 +92,19 @@ func registerNewVolume(rootCipherDir string, masterkey []byte, cf *configfile.Co
 	if newVolume.plainTextNames {
 		ivLen = 0
 	}
-	// New empty caches
 	newVolume.dirCache = dirCache{ivLen: ivLen}
-	newVolume.file_handles = make(map[int]File)
-	newVolume.fileIDs = make(map[int][]byte)
 
 	//find unused volumeID
 	volumeID := -1
 	c := 0
 	for volumeID == -1 {
-		_, ok := OpenedVolumes[c]
+		_, ok := OpenedVolumes.Load(c)
 		if !ok {
 			volumeID = c
 		}
 		c++
 	}
-	if OpenedVolumes == nil {
-		OpenedVolumes = make(map[int]*Volume)
-	}
-	OpenedVolumes[volumeID] = &newVolume
+	OpenedVolumes.Store(volumeID, &newVolume)
 	return volumeID
 }
 
@@ -117,21 +124,23 @@ func gcf_init(rootCipherDir string, password, givenScryptHash, returnedScryptHas
 
 //export gcf_close
 func gcf_close(volumeID int) {
-	volume, ok := OpenedVolumes[volumeID]
+	value, ok := OpenedVolumes.Load(volumeID)
 	if !ok {
 		return
 	}
+	volume := value.(*Volume)
 	volume.cryptoCore.Wipe()
-	for handleID := range volume.file_handles {
-		gcf_close_file(volumeID, handleID)
-	}
+	volume.file_handles.Range(func (handleID, _ interface {}) bool {
+		gcf_close_file(volumeID, handleID.(int))
+		return true
+	})
 	volume.dirCache.Clear()
-	delete(OpenedVolumes, volumeID)
+	OpenedVolumes.Delete(volumeID)
 }
 
 //export gcf_is_closed
 func gcf_is_closed(volumeID int) bool {
-	_, ok := OpenedVolumes[volumeID]
+	_, ok := OpenedVolumes.Load(volumeID)
 	return !ok
 }