Commit 17756c7c authored by Han-Wen Nienhuys's avatar Han-Wen Nienhuys Committed by Han-Wen Nienhuys

fs: buffer error in directory reads

Previously, if a DirStream.Next() call would return error, we would
return the buffered data as well as the errno code in the same
request.

This would confuse the kernel, who interprets this as an invalid
response, returning syscall.EIO to the caller.

To fix this, also buffer the errno as overflow entry in the fileEntry
struct. When we encounter an error, return thus far read entries with
success. The following read contains just the error.

For testing this properly, loopbackDirStream (which reads directory
contents) must also buffer error returns.

Change-Id: Ideb8ea6f540de9189a0f366efca0db2205e5dec3
parent 176ecae2
...@@ -33,6 +33,8 @@ type fileEntry struct { ...@@ -33,6 +33,8 @@ type fileEntry struct {
dirStream DirStream dirStream DirStream
hasOverflow bool hasOverflow bool
overflow fuse.DirEntry overflow fuse.DirEntry
overflowErrno syscall.Errno
// dirOffset is the current location in the directory (see `telldir(3)`). // dirOffset is the current location in the directory (see `telldir(3)`).
// The value is equivalent to `d_off` (see `getdents(2)`) of the last // The value is equivalent to `d_off` (see `getdents(2)`) of the last
// directory entry sent to the kernel so far. // directory entry sent to the kernel so far.
...@@ -999,18 +1001,31 @@ func (b *rawBridge) ReadDir(cancel <-chan struct{}, input *fuse.ReadIn, out *fus ...@@ -999,18 +1001,31 @@ func (b *rawBridge) ReadDir(cancel <-chan struct{}, input *fuse.ReadIn, out *fus
} }
if f.hasOverflow { if f.hasOverflow {
if f.overflowErrno != 0 {
return errnoToStatus(f.overflowErrno)
}
f.hasOverflow = false
// always succeeds. // always succeeds.
out.AddDirEntry(f.overflow) out.AddDirEntry(f.overflow)
f.hasOverflow = false
f.dirOffset++ f.dirOffset++
} }
first := true
for f.dirStream.HasNext() { for f.dirStream.HasNext() {
e, errno := f.dirStream.Next() e, errno := f.dirStream.Next()
if errno != 0 { if errno != 0 {
if first {
return errnoToStatus(errno) return errnoToStatus(errno)
} else {
f.hasOverflow = true
f.overflowErrno = errno
return fuse.OK
} }
}
first = false
if !out.AddDirEntry(e) { if !out.AddDirEntry(e) {
f.overflow = e f.overflow = e
f.hasOverflow = true f.hasOverflow = true
...@@ -1036,20 +1051,30 @@ func (b *rawBridge) ReadDirPlus(cancel <-chan struct{}, input *fuse.ReadIn, out ...@@ -1036,20 +1051,30 @@ func (b *rawBridge) ReadDirPlus(cancel <-chan struct{}, input *fuse.ReadIn, out
} }
ctx := &fuse.Context{Caller: input.Caller, Cancel: cancel} ctx := &fuse.Context{Caller: input.Caller, Cancel: cancel}
first := true
for f.dirStream.HasNext() || f.hasOverflow { for f.dirStream.HasNext() || f.hasOverflow {
var e fuse.DirEntry var e fuse.DirEntry
var errno syscall.Errno var errno syscall.Errno
if f.hasOverflow { if f.hasOverflow {
e = f.overflow e = f.overflow
errno = f.overflowErrno
f.hasOverflow = false f.hasOverflow = false
} else { } else {
e, errno = f.dirStream.Next() e, errno = f.dirStream.Next()
} }
if errno != 0 { if errno != 0 {
if first {
return errnoToStatus(errno) return errnoToStatus(errno)
} else {
f.overflowErrno = errno
f.hasOverflow = true
return fuse.OK
}
} }
first = false
entryOut := out.AddDirLookupEntry(e) entryOut := out.AddDirLookupEntry(e)
if entryOut == nil { if entryOut == nil {
......
// Copyright 2023 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 fs
import (
"context"
"fmt"
"syscall"
"testing"
"github.com/hanwen/go-fuse/v2/fuse"
)
type dirStreamErrorNode struct {
Inode
}
var _ = (NodeReaddirer)((*dirStreamErrorNode)(nil))
func (n *dirStreamErrorNode) Readdir(ctx context.Context) (DirStream, syscall.Errno) {
return &errDirStream{}, 0
}
type errDirStream struct {
num int
}
func (ds *errDirStream) HasNext() bool {
return ds.num < 2
}
func (ds *errDirStream) Next() (fuse.DirEntry, syscall.Errno) {
ds.num++
if ds.num == 1 {
return fuse.DirEntry{
Mode: fuse.S_IFREG,
Name: "first",
Ino: 2,
}, 0
}
if ds.num == 2 {
return fuse.DirEntry{
Mode: fuse.S_IFREG,
Name: "last",
Ino: 3,
}, syscall.EKEYEXPIRED
}
panic("boom")
}
func (ds *errDirStream) Close() {
}
func TestDirStreamError(t *testing.T) {
for _, disableReaddirplus := range []bool{false, true} {
t.Run(fmt.Sprintf("disableReaddirplus=%v", disableReaddirplus),
func(t *testing.T) {
root := &dirStreamErrorNode{}
opts := Options{}
opts.DisableReadDirPlus = disableReaddirplus
mnt, _ := testMount(t, root, &opts)
ds, errno := NewLoopbackDirStream(mnt)
if errno != 0 {
t.Fatalf("NewLoopbackDirStream: %v", errno)
}
defer ds.Close()
if e, errno := ds.Next(); errno != 0 {
t.Errorf("ds.Next: %v", errno)
} else if e.Name != "first" {
t.Errorf("got %q want 'first'", e.Name)
}
if _, errno := ds.Next(); errno != syscall.EKEYEXPIRED {
t.Errorf("got errno %v, want EKEYEXPIRED", errno)
}
})
}
}
...@@ -15,6 +15,7 @@ import ( ...@@ -15,6 +15,7 @@ import (
type loopbackDirStream struct { type loopbackDirStream struct {
buf []byte buf []byte
todo []byte todo []byte
todoErrno syscall.Errno
// Protects fd so we can guard against double close // Protects fd so we can guard against double close
mu sync.Mutex mu sync.Mutex
...@@ -52,7 +53,7 @@ func (ds *loopbackDirStream) Close() { ...@@ -52,7 +53,7 @@ func (ds *loopbackDirStream) Close() {
func (ds *loopbackDirStream) HasNext() bool { func (ds *loopbackDirStream) HasNext() bool {
ds.mu.Lock() ds.mu.Lock()
defer ds.mu.Unlock() defer ds.mu.Unlock()
return len(ds.todo) > 0 return len(ds.todo) > 0 || ds.todoErrno != 0
} }
// Like syscall.Dirent, but without the [256]byte name. // Like syscall.Dirent, but without the [256]byte name.
...@@ -68,6 +69,10 @@ func (ds *loopbackDirStream) Next() (fuse.DirEntry, syscall.Errno) { ...@@ -68,6 +69,10 @@ func (ds *loopbackDirStream) Next() (fuse.DirEntry, syscall.Errno) {
ds.mu.Lock() ds.mu.Lock()
defer ds.mu.Unlock() defer ds.mu.Unlock()
if ds.todoErrno != 0 {
return fuse.DirEntry{}, ds.todoErrno
}
// We can't use syscall.Dirent here, because it declares a // We can't use syscall.Dirent here, because it declares a
// [256]byte name, which may run beyond the end of ds.todo. // [256]byte name, which may run beyond the end of ds.todo.
// when that happens in the race detector, it causes a panic // when that happens in the race detector, it causes a panic
...@@ -99,9 +104,10 @@ func (ds *loopbackDirStream) load() syscall.Errno { ...@@ -99,9 +104,10 @@ func (ds *loopbackDirStream) load() syscall.Errno {
} }
n, err := syscall.Getdents(ds.fd, ds.buf) n, err := syscall.Getdents(ds.fd, ds.buf)
if err != nil { if n < 0 {
return ToErrno(err) n = 0
} }
ds.todo = ds.buf[:n] ds.todo = ds.buf[:n]
ds.todoErrno = ToErrno(err)
return OK return OK
} }
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