Commit 402331d4 authored by Kirill Smelkov's avatar Kirill Smelkov Committed by Han-Wen Nienhuys

nodefs: Don't crash on handleless WithFlags opens

Since https://git.kernel.org/linus/7678ac5061 FUSE filesystems were allowed to
return ENOSYS from open and, open seeing this, the kernel will not send any
open request anymore and just use Fh=0 for all opened files. This is handy for
filesystems that don't have any per-opened-file state.

However if there is at least one node for which opened files should have
their own state, it is not correct to return ENOSYS from open _ever_, as that
would prevent the kernel to call open on all nodes - in particular on the node
where open needs to create a state associated with the file handle(*).

It is already explicitly documented that Node.Open can return File=nil
in which case filesystem operations like Read and Write will be called
on the node directly. This way a filesystem could try to simulate an
Open that was returning ENOSYS with Open that returns File=nil. However
it is not the same as ENOSYS open also prevents the kernel from dropping
the file cache:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/fuse/file.c?id=v5.0-rc3-241-g7c2614bf7a1f#n131

From this point of view Opens that were returning ENOSYS should be
replaced with Open that returns WithFlags{File=nil, flags=FOPEN_KEEP_CACHE}.

Unfortunately if one tries to do that, go-fuse segfaults as the test
included in this patch demonstrates:

	13:36:53.238706 rx 13: LOOKUP i1 ["world.txt"] 10b
	13:36:53.238745 tx 13:     OK, {i4 g3 tE=1s tA=1s {M0100644 SZ=5 L=1 1000:1000 B0*0 i0:4 A 0.000000 M 0.000000 C 0.000000}}
	13:36:53.238780 rx 14: OPEN i4 {O_RDONLY,0x8000}
	13:36:53.238791 tx 14:     OK, {Fh 2 CACHE}				<-- NOTE Fh != 0
	13:36:53.238706 rx 12: RELEASE i3 {Fh 0 NONBLOCK,0x8000  L0}
	13:36:53.238804 tx 12:     OK
	13:36:53.238823 rx 15: READ i4 {Fh 2 [0 +4096)  L 0 NONBLOCK,0x8000}
	13:36:53.238830 tx 15:     OK,  5b data "world"
	13:36:53.238846 rx 16: GETATTR i4 {Fh 2}
	13:36:53.238865 tx 16:     OK, {tA=1s {M0100644 SZ=5 L=1 1000:1000 B0*0 i0:4 A 0.000000 M 0.000000 C 0.000000}}
	13:36:53.238879 rx 17: FLUSH i4 {Fh 2}
	panic: runtime error: invalid memory address or nil pointer dereference
	[signal SIGSEGV: segmentation violation code=0x1 addr=0x30 pc=0x539f9f]

	goroutine 6 [running]:
	github.com/hanwen/go-fuse/fuse/nodefs.(*rawBridge).Flush(0xc000010960, 0xc000104180, 0x0)
	        /home/kirr/src/neo/src/github.com/hanwen/go-fuse/fuse/nodefs/fsops.go:490 +0x6f
	github.com/hanwen/go-fuse/fuse.doFlush(0xc0000de000, 0xc000104000)
	        /home/kirr/src/neo/src/github.com/hanwen/go-fuse/fuse/opcode.go:373 +0x42
	github.com/hanwen/go-fuse/fuse.(*Server).handleRequest(0xc0000de000, 0xc000104000, 0xc000104000)
	        /home/kirr/src/neo/src/github.com/hanwen/go-fuse/fuse/server.go:431 +0x26b
	github.com/hanwen/go-fuse/fuse.(*Server).loop(0xc0000de000, 0x0)
	        /home/kirr/src/neo/src/github.com/hanwen/go-fuse/fuse/server.go:403 +0x18f
	github.com/hanwen/go-fuse/fuse.(*Server).Serve(0xc0000de000)
	        /home/kirr/src/neo/src/github.com/hanwen/go-fuse/fuse/server.go:331 +0x6d
	created by github.com/hanwen/go-fuse/fuse/test.TestNoFile
	        /home/kirr/src/neo/src/github.com/hanwen/go-fuse/fuse/test/nofile_test.go:73 +0x442

Fix it by teaching registerFileHandle not to register any handle at all
and return Fh=0, if the inner file (file itself, or the file that was wrapped
with WithFlags) is nil.

After the patch the trace for world.txt open/read (that is opened with
WithFlags{File=nil, Flags=FOPEN_KEEP_CACHE} is as follows.

	14:59:31.714048 rx 13: LOOKUP i1 ["world.txt"] 10b
	14:59:31.714062 tx 13:     OK, {i4 g3 tE=1s tA=1s {M0100644 SZ=5 L=1 1000:1000 B0*0 i0:4 A 0.000000 M 0.000000 C 0.000000}}
	14:59:31.714081 rx 14: OPEN i4 {O_RDONLY,0x8000}
	14:59:31.714091 tx 14:     OK, {Fh 0 CACHE}				<-- NOTE Fh = 0
	14:59:31.714123 rx 15: READ i4 {Fh 0 [0 +4096)  L 0 NONBLOCK,0x8000}
	14:59:31.714132 tx 15:     OK,  5b data "world"
	14:59:31.714150 rx 16: GETATTR i4 {Fh 0}
	14:59:31.714159 tx 16:     OK, {tA=1s {M0100644 SZ=5 L=1 1000:1000 B0*0 i0:4 A 0.000000 M 0.000000 C 0.000000}}
	14:59:31.714181 rx 17: FLUSH i4 {Fh 0}
	14:59:31.714186 tx 17:     OK
	14:59:31.714202 rx 18: RELEASE i4 {Fh 0 NONBLOCK,0x8000  L0}
	14:59:31.714208 tx 18:     OK

(*) my particular case is filesystem where many nodes are just data, but
additionally there are files that behave like sockets - for every client
who opens such file the filesystem establishes separate bidirectional
channel for control exchange via that stream:

https://lab.nexedi.com/kirr/wendelin.core/blob/a50da567fd/wcfs/misc.go#L205
parent f45e18d6
...@@ -116,8 +116,24 @@ func (m *fileSystemMount) unregisterFileHandle(handle uint64, node *Inode) *open ...@@ -116,8 +116,24 @@ func (m *fileSystemMount) unregisterFileHandle(handle uint64, node *Inode) *open
return opened return opened
} }
func (m *fileSystemMount) registerFileHandle(node *Inode, dir *connectorDir, f File, flags uint32) (uint64, *openedFile) { // registerFileHandle registers f or dir to have a handle.
node.openFilesMutex.Lock() //
// The handle is then used as file-handle in communications with kernel.
//
// If dir != nil the handle is registered for OpenDir and the inner file (see
// below) must be nil. If dir = nil the handle is registered for regular open &
// friends.
//
// f can be nil, or a WithFlags that leads to File=nil. For !OpenDir, if that
// is the case, returned handle will be 0 to indicate a handleless open, and
// the filesystem operations on the opened file will be routed to be served by
// the node.
//
// other arguments:
//
// node - Inode for which f or dir were opened,
// flags - file open flags, like O_RDWR.
func (m *fileSystemMount) registerFileHandle(node *Inode, dir *connectorDir, f File, flags uint32) (handle uint64, opened *openedFile) {
b := &openedFile{ b := &openedFile{
dir: dir, dir: dir,
WithFlags: WithFlags{ WithFlags: WithFlags{
...@@ -138,11 +154,23 @@ func (m *fileSystemMount) registerFileHandle(node *Inode, dir *connectorDir, f F ...@@ -138,11 +154,23 @@ func (m *fileSystemMount) registerFileHandle(node *Inode, dir *connectorDir, f F
f = withFlags.File f = withFlags.File
} }
// don't allow both dir and file
if dir != nil && b.WithFlags.File != nil {
panic("registerFileHandle: both dir and file are set.")
}
if b.WithFlags.File == nil && dir == nil {
// it was just WithFlags{...}, but the file itself is nil
return 0, b
}
if b.WithFlags.File != nil { if b.WithFlags.File != nil {
b.WithFlags.File.SetInode(node) b.WithFlags.File.SetInode(node)
} }
node.openFilesMutex.Lock()
node.openFiles = append(node.openFiles, b) node.openFiles = append(node.openFiles, b)
handle, _ := m.openFiles.Register(&b.handled) handle, _ = m.openFiles.Register(&b.handled)
node.openFilesMutex.Unlock() node.openFilesMutex.Unlock()
return handle, b return handle, b
} }
......
...@@ -185,7 +185,7 @@ func (c *rawBridge) ReadDirPlus(input *fuse.ReadIn, out *fuse.DirEntryList) fuse ...@@ -185,7 +185,7 @@ func (c *rawBridge) ReadDirPlus(input *fuse.ReadIn, out *fuse.DirEntryList) fuse
func (c *rawBridge) Open(input *fuse.OpenIn, out *fuse.OpenOut) (status fuse.Status) { func (c *rawBridge) Open(input *fuse.OpenIn, out *fuse.OpenOut) (status fuse.Status) {
node := c.toInode(input.NodeId) node := c.toInode(input.NodeId)
f, code := node.fsInode.Open(input.Flags, &input.Context) f, code := node.fsInode.Open(input.Flags, &input.Context)
if !code.Ok() || f == nil { if !code.Ok() {
return code return code
} }
h, opened := node.mount.registerFileHandle(node, nil, f, input.Flags) h, opened := node.mount.registerFileHandle(node, nil, f, input.Flags)
......
// Copyright 2019 the Go-FUSE Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
package test
import (
"io/ioutil"
"os"
"sync/atomic"
"testing"
"github.com/hanwen/go-fuse/fuse"
"github.com/hanwen/go-fuse/fuse/nodefs"
"github.com/hanwen/go-fuse/internal/testutil"
)
// exercise functionality when open returns 0 file handle.
// NoFileNode is DataNode for which open returns File=nil.
//
// In other words all operations on a file opened from NoFileNode, are routed
// to NoFileNode itself.
type NoFileNode struct {
*DataNode
flags uint32 // FUSE flags for open, e.g. FOPEN_KEEP_CACHE
nopen int32 // #Open called on us
}
// NewNoFileNode creates new file node for which Open will return File=nil, and
// if flags !=0, will wrap it into WithFlags.
func NewNoFileNode(data []byte, flags uint32) *NoFileNode {
return &NoFileNode{
DataNode: NewDataNode(data),
flags: flags,
nopen: 0,
}
}
func (d *NoFileNode) Open(flags uint32, context *fuse.Context) (nodefs.File, fuse.Status) {
var f nodefs.File = nil
if d.flags != 0 {
f = &nodefs.WithFlags{
File: f,
FuseFlags: d.flags,
}
}
atomic.AddInt32(&d.nopen, +1)
return f, fuse.OK
}
func TestNoFile(t *testing.T) {
dir := testutil.TempDir()
defer func() {
err := os.Remove(dir)
if err != nil {
t.Fatal(err)
}
}()
// setup a filesystem with 2 files:
//
// open(hello.txt) -> nil
// open(world.txt) -> WithFlags(nil, FOPEN_KEEP_CACHE)
root := nodefs.NewDefaultNode()
opts := nodefs.NewOptions()
opts.Debug = testutil.VerboseTest()
srv, _, err := nodefs.MountRoot(dir, root, opts)
if err != nil {
t.Fatal(err)
}
hello := NewNoFileNode([]byte("hello"), 0)
world := NewNoFileNode([]byte("world"), fuse.FOPEN_KEEP_CACHE)
root.Inode().NewChild("hello.txt", false, hello)
root.Inode().NewChild("world.txt", false, world)
go srv.Serve()
if err := srv.WaitMount(); err != nil {
t.Fatal("WaitMount", err)
}
defer func() {
err := srv.Unmount()
if err != nil {
t.Fatal(err)
}
}()
// assertOpenRead asserts that file @ path reads as dataOK, and that
// corresponding Open is called on the filesystem.
assertOpenRead := func(path string, node *NoFileNode, dataOK string) {
t.Helper()
nopenPre := atomic.LoadInt32(&node.nopen)
v, err := ioutil.ReadFile(dir + path)
if err != nil {
t.Fatalf("%s: read: %s", path, err)
}
if string(v) != dataOK {
t.Fatalf("%s: read: got %q ; want %q", path, v, dataOK)
}
// make sure that path.Open() was called.
//
// this can be not the case if the filesystem has a node with
// Open that returns ENOSYS - then the kernel won't ever call
// open for all other nodes.
nopen := atomic.LoadInt32(&node.nopen)
if nopen == nopenPre {
t.Fatalf("%s: read: open was not called", path)
}
}
// make sure all nodes can be open/read.
assertOpenRead("/hello.txt", hello, "hello")
assertOpenRead("/world.txt", world, "world")
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment