From 9ab11aa4d775f7c1242e8b044cc2f8957cc2c784 Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Mon, 1 May 2017 17:49:37 +0200 Subject: [PATCH] fusefrontend: drop writeOnly flag We do not have to track the writeOnly status because the kernel will not forward read requests on a write-only FD to us anyway. I have verified this behavoir manually on a 4.10.8 kernel and also added a testcase. --- internal/fusefrontend/file.go | 10 +--------- internal/fusefrontend/fs.go | 23 ++++++++++++----------- tests/defaults/main_test.go | 15 +++++++++++++++ 3 files changed, 28 insertions(+), 20 deletions(-) diff --git a/internal/fusefrontend/file.go b/internal/fusefrontend/file.go index 92b1496..0a8c8aa 100644 --- a/internal/fusefrontend/file.go +++ b/internal/fusefrontend/file.go @@ -35,8 +35,6 @@ type file struct { // Every FUSE entrypoint should RLock(). The only user of Lock() is // Release(), which closes the fd and sets "released" to true. fdLock sync.RWMutex - // Was the file opened O_WRONLY? - writeOnly bool // Content encryption helper contentEnc *contentenc.ContentEnc // Device and inode number uniquely identify the backing file @@ -59,7 +57,7 @@ type file struct { } // NewFile returns a new go-fuse File instance. -func NewFile(fd *os.File, writeOnly bool, fs *FS) (nodefs.File, fuse.Status) { +func NewFile(fd *os.File, fs *FS) (nodefs.File, fuse.Status) { var st syscall.Stat_t err := syscall.Fstat(int(fd.Fd()), &st) if err != nil { @@ -71,7 +69,6 @@ func NewFile(fd *os.File, writeOnly bool, fs *FS) (nodefs.File, fuse.Status) { return &file{ fd: fd, - writeOnly: writeOnly, contentEnc: fs.contentEnc, qIno: qi, fileTableEntry: e, @@ -229,11 +226,6 @@ func (f *file) Read(buf []byte, off int64) (resultData fuse.ReadResult, code fus tlog.Debug.Printf("ino%d: FUSE Read: offset=%d length=%d", f.qIno.Ino, len(buf), off) - if f.writeOnly { - tlog.Warn.Printf("ino%d: Tried to read from write-only file", f.qIno.Ino) - return nil, fuse.EBADF - } - if f.fs.args.SerializeReads { serialize_reads.Wait(off, len(buf)) } diff --git a/internal/fusefrontend/fs.go b/internal/fusefrontend/fs.go index b38e362..5e5157c 100644 --- a/internal/fusefrontend/fs.go +++ b/internal/fusefrontend/fs.go @@ -80,17 +80,18 @@ func (fs *FS) GetAttr(name string, context *fuse.Context) (*fuse.Attr, fuse.Stat return a, status } -// We always need read access to do read-modify-write cycles -func (fs *FS) mangleOpenFlags(flags uint32) (newFlags int, writeOnly bool) { +// mangleOpenFlags is used by Create() and Open() to convert the open flags the user +// wants to the flags we internally use to open the backing file. +func (fs *FS) mangleOpenFlags(flags uint32) (newFlags int) { newFlags = int(flags) + // Convert WRONLY to RDWR. We always need read access to do read-modify-write cycles. if newFlags&os.O_WRONLY > 0 { - writeOnly = true newFlags = newFlags ^ os.O_WRONLY | os.O_RDWR } // We also cannot open the file in append mode, we need to seek back for RMW newFlags = newFlags &^ os.O_APPEND - return newFlags, writeOnly + return newFlags } // Open implements pathfs.Filesystem. @@ -98,19 +99,19 @@ func (fs *FS) Open(path string, flags uint32, context *fuse.Context) (fuseFile n if fs.isFiltered(path) { return nil, fuse.EPERM } - iflags, writeOnly := fs.mangleOpenFlags(flags) + newFlags := fs.mangleOpenFlags(flags) cPath, err := fs.getBackingPath(path) if err != nil { tlog.Debug.Printf("Open: getBackingPath: %v", err) return nil, fuse.ToStatus(err) } tlog.Debug.Printf("Open: %s", cPath) - f, err := os.OpenFile(cPath, iflags, 0666) + f, err := os.OpenFile(cPath, newFlags, 0666) if err != nil { return nil, fuse.ToStatus(err) } - return NewFile(f, writeOnly, fs) + return NewFile(f, fs) } // Create implements pathfs.Filesystem. @@ -118,7 +119,7 @@ func (fs *FS) Create(path string, flags uint32, mode uint32, context *fuse.Conte if fs.isFiltered(path) { return nil, fuse.EPERM } - iflags, writeOnly := fs.mangleOpenFlags(flags) + newFlags := fs.mangleOpenFlags(flags) cPath, err := fs.getBackingPath(path) if err != nil { return nil, fuse.ToStatus(err) @@ -144,7 +145,7 @@ func (fs *FS) Create(path string, flags uint32, mode uint32, context *fuse.Conte // Create content var fdRaw int - fdRaw, err = syscallcompat.Openat(int(dirfd.Fd()), cName, iflags|os.O_CREATE, mode) + fdRaw, err = syscallcompat.Openat(int(dirfd.Fd()), cName, newFlags|os.O_CREATE, mode) if err != nil { nametransform.DeleteLongName(dirfd, cName) return nil, fuse.ToStatus(err) @@ -152,7 +153,7 @@ func (fs *FS) Create(path string, flags uint32, mode uint32, context *fuse.Conte fd = os.NewFile(uintptr(fdRaw), cName) } else { // Normal (short) file name - fd, err = os.OpenFile(cPath, iflags|os.O_CREATE, os.FileMode(mode)) + fd, err = os.OpenFile(cPath, newFlags|os.O_CREATE, os.FileMode(mode)) if err != nil { return nil, fuse.ToStatus(err) } @@ -164,7 +165,7 @@ func (fs *FS) Create(path string, flags uint32, mode uint32, context *fuse.Conte tlog.Warn.Printf("Create: fd.Chown failed: %v", err) } } - return NewFile(fd, writeOnly, fs) + return NewFile(fd, fs) } // Chmod implements pathfs.Filesystem. diff --git a/tests/defaults/main_test.go b/tests/defaults/main_test.go index 52ca727..8c1b7e3 100644 --- a/tests/defaults/main_test.go +++ b/tests/defaults/main_test.go @@ -116,3 +116,18 @@ func TestOpenTruncateRead(t *testing.T) { t.Fatalf("wrong content: %s", string(content)) } } + +// TestWORead tries to read from a write-only file. +func TestWORead(t *testing.T) { + fn := test_helpers.DefaultPlainDir + "/TestWORead" + fd, err := os.OpenFile(fn, os.O_CREATE|os.O_WRONLY, 0600) + if err != nil { + t.Fatal(err) + } + defer fd.Close() + buf := make([]byte, 10) + _, err = fd.Read(buf) + if err == nil { + t.Error("Reading from write-only file should fail, but did not") + } +}