diff options
| author | Jakob Unterwurzacher | 2017-05-01 17:49:37 +0200 | 
|---|---|---|
| committer | Jakob Unterwurzacher | 2017-05-01 17:49:37 +0200 | 
| commit | 9ab11aa4d775f7c1242e8b044cc2f8957cc2c784 (patch) | |
| tree | f4f302b3573dff151b2290d6f1a2d9436cd24e57 | |
| parent | 514f515dd7196e26ca8df6886ac4a34e928e50dd (diff) | |
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.
| -rw-r--r-- | internal/fusefrontend/file.go | 10 | ||||
| -rw-r--r-- | internal/fusefrontend/fs.go | 23 | ||||
| -rw-r--r-- | 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") +	} +} | 
