Skip to content

Commit

Permalink
fix getxattr and listxattr (#72)
Browse files Browse the repository at this point in the history
previously, this will fail if /mnt/file doesn't have an xattr:

```
listxattr("/mnt/file", 0x7fe8b3686830, 256) = -1 EIO (Input/output error)
```

We should be returning the actual size only if the input size is
zero. Related issue is if the filesystem returns ERANGE, we should
propagate that error instead of returning the actual size.

Replaced go-xattr usage with x/sys/unix so we can test this.
  • Loading branch information
kahing authored and stapelberg committed Dec 11, 2019
1 parent 4ee1cf7 commit e7bcad2
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 43 deletions.
15 changes: 2 additions & 13 deletions conversions.go
Original file line number Diff line number Diff line change
Expand Up @@ -603,17 +603,6 @@ func (c *Connection) kernelResponse(
if opErr != nil {
handled := false

if opErr == syscall.ERANGE {
switch o := op.(type) {
case *fuseops.GetXattrOp:
writeXattrSize(m, uint32(o.BytesRead))
handled = true
case *fuseops.ListXattrOp:
writeXattrSize(m, uint32(o.BytesRead))
handled = true
}
}

if !handled {
m.OutHeader().Error = -int32(syscall.EIO)
if errno, ok := opErr.(syscall.Errno); ok {
Expand Down Expand Up @@ -792,14 +781,14 @@ func (c *Connection) kernelResponseForOp(
// convertInMessage already set up the destination buffer to be at the end
// of the out message. We need only shrink to the right size based on how
// much the user read.
if o.BytesRead == 0 {
if len(o.Dst) == 0 {
writeXattrSize(m, uint32(o.BytesRead))
} else {
m.ShrinkTo(buffer.OutMessageHeaderSize + o.BytesRead)
}

case *fuseops.ListXattrOp:
if o.BytesRead == 0 {
if len(o.Dst) == 0 {
writeXattrSize(m, uint32(o.BytesRead))
} else {
m.ShrinkTo(buffer.OutMessageHeaderSize + o.BytesRead)
Expand Down
11 changes: 6 additions & 5 deletions samples/memfs/memfs.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/jacobsa/fuse/fuseops"
"github.com/jacobsa/fuse/fuseutil"
"github.com/jacobsa/syncutil"
"golang.org/x/sys/unix"
)

type memFS struct {
Expand Down Expand Up @@ -679,7 +680,7 @@ func (fs *memFS) GetXattr(ctx context.Context,
op.BytesRead = len(value)
if len(op.Dst) >= len(value) {
copy(op.Dst, value)
} else {
} else if len(op.Dst) != 0 {
err = syscall.ERANGE
}
} else {
Expand All @@ -703,8 +704,8 @@ func (fs *memFS) ListXattr(ctx context.Context,
if err == nil && len(dst) >= keyLen {
copy(dst, key)
dst = dst[keyLen:]
} else {
err = syscall.ERANGE
} else if len(op.Dst) != 0 {
return syscall.ERANGE
}
op.BytesRead += keyLen
}
Expand Down Expand Up @@ -735,11 +736,11 @@ func (fs *memFS) SetXattr(ctx context.Context,
_, ok := inode.xattrs[op.Name]

switch op.Flags {
case 0x1:
case unix.XATTR_CREATE:
if ok {
err = fuse.EEXIST
}
case 0x2:
case unix.XATTR_REPLACE:
if !ok {
err = fuse.ENOATTR
}
Expand Down
68 changes: 43 additions & 25 deletions samples/memfs/memfs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import (
"github.com/jacobsa/fuse/samples/memfs"
. "github.com/jacobsa/oglematchers"
. "github.com/jacobsa/ogletest"
"github.com/kahing/go-xattr"
"golang.org/x/sys/unix"
)

func TestMemFS(t *testing.T) { RunTests(t) }
Expand Down Expand Up @@ -1792,60 +1792,78 @@ func (t *MemFSTest) RenameNonExistentFile() {

func (t *MemFSTest) NoXattrs() {
var err error
var sz int
var smallBuf [1]byte

// Create a file.
filePath := path.Join(t.Dir, "foo")
err = ioutil.WriteFile(filePath, []byte("taco"), 0400)
AssertEq(nil, err)

// List xattr names.
names, err := xattr.List(filePath)
sz, err = unix.Listxattr(filePath, nil)
AssertEq(nil, err)
ExpectThat(names, ElementsAre())
AssertEq(0, sz)

// Attempt to read a non-existent xattr.
_, err = xattr.Getxattr(filePath, "foo", nil)
_, err = unix.Getxattr(filePath, "foo", nil)
ExpectEq(fuse.ENOATTR, err)

// Attempt to read a non-existent xattr with a buf.
_, err = unix.Getxattr(filePath, "foo", smallBuf[:])
ExpectEq(fuse.ENOATTR, err)

// List xattr names with a buf.
sz, err = unix.Listxattr(filePath, smallBuf[:])
AssertEq(nil, err)
ExpectEq(0, sz)
}

func (t *MemFSTest) SetXAttr() {
var err error
var sz int
var buf [1024]byte

// Create a file.
filePath := path.Join(t.Dir, "foo")
err = ioutil.WriteFile(filePath, []byte("taco"), 0600)
AssertEq(nil, err)

err = xattr.Setxattr(filePath, "foo", []byte("bar"), xattr.REPLACE)
err = unix.Setxattr(filePath, "foo", []byte("bar"), unix.XATTR_REPLACE)
AssertEq(fuse.ENOATTR, err)

err = xattr.Setxattr(filePath, "foo", []byte("bar"), xattr.CREATE)
err = unix.Setxattr(filePath, "foo", []byte("bar"), unix.XATTR_CREATE)
AssertEq(nil, err)

value, err := xattr.Get(filePath, "foo")
AssertEq(nil, err)
AssertEq("bar", string(value))
// List xattr with a buf that is too small.
_, err = unix.Listxattr(filePath, buf[:1])
ExpectEq(unix.ERANGE, err)

err = xattr.Setxattr(filePath, "foo", []byte("hello world"), xattr.REPLACE)
// List xattr to ask for name size.
sz, err = unix.Listxattr(filePath, nil)
AssertEq(nil, err)
AssertEq(4, sz)

value, err = xattr.Get(filePath, "foo")
// List xattr names.
sz, err = unix.Listxattr(filePath, buf[:sz])
AssertEq(nil, err)
AssertEq("hello world", string(value))
AssertEq(4, sz)
AssertEq("foo\000", string(buf[:sz]))

names, err := xattr.List(filePath)
AssertEq(nil, err)
AssertEq(1, len(names))
AssertEq("foo", names[0])
// Read xattr with a buf that is too small.
_, err = unix.Getxattr(filePath, "foo", buf[:1])
ExpectEq(unix.ERANGE, err)

err = xattr.Setxattr(filePath, "bar", []byte("hello world"), 0x0)
// Read xattr to ask for value size.
sz, err = unix.Getxattr(filePath, "foo", nil)
AssertEq(nil, err)
AssertEq(3, sz)

names, err = xattr.List(filePath)
// Read xattr value.
sz, err = unix.Getxattr(filePath, "foo", buf[:sz])
AssertEq(nil, err)
AssertEq(2, len(names))
ExpectThat(names, Contains("foo"))
ExpectThat(names, Contains("bar"))
AssertEq(3, sz)
AssertEq("bar", string(buf[:sz]))
}

func (t *MemFSTest) RemoveXAttr() {
Expand All @@ -1856,16 +1874,16 @@ func (t *MemFSTest) RemoveXAttr() {
err = ioutil.WriteFile(filePath, []byte("taco"), 0600)
AssertEq(nil, err)

err = xattr.Removexattr(filePath, "foo")
err = unix.Removexattr(filePath, "foo")
AssertEq(fuse.ENOATTR, err)

err = xattr.Setxattr(filePath, "foo", []byte("bar"), xattr.CREATE)
err = unix.Setxattr(filePath, "foo", []byte("bar"), unix.XATTR_CREATE)
AssertEq(nil, err)

err = xattr.Removexattr(filePath, "foo")
err = unix.Removexattr(filePath, "foo")
AssertEq(nil, err)

_, err = xattr.Getxattr(filePath, "foo", nil)
_, err = unix.Getxattr(filePath, "foo", nil)
AssertEq(fuse.ENOATTR, err)
}

Expand Down

0 comments on commit e7bcad2

Please sign in to comment.