From 985d8523434814cd9cba61b666a1f888bdcbaded Mon Sep 17 00:00:00 2001 From: Hardcore Sushi Date: Wed, 20 Apr 2022 21:17:32 +0200 Subject: [PATCH] Thread safety --- README.md | 2 +- common_ops.go | 6 ++- dircache.go | 18 +++++++++ directory.go | 22 ++++++++--- file.go | 106 +++++++++++++++++++++++++++++++++----------------- volume.go | 49 +++++++++++++---------- 6 files changed, 139 insertions(+), 64 deletions(-) diff --git a/README.md b/README.md index 0a1db84..4c523c7 100644 --- a/README.md +++ b/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 ! diff --git a/common_ops.go b/common_ops.go index 0d96aa5..06b58fa 100644 --- a/common_ops.go +++ b/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 diff --git a/dircache.go b/dircache.go index 508d38d..09771f4 100644 --- a/dircache.go +++ b/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 + } + } +} diff --git a/directory.go b/directory.go index 30259f8..d0093e7 100644 --- a/directory.go +++ b/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 } diff --git a/file.go b/file.go index d5be43e..6b9e470 100644 --- a/file.go +++ b/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 diff --git a/volume.go b/volume.go index 108de9e..02488a9 100644 --- a/volume.go +++ b/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 }