From e7bcad2083673682525e22dd8d90efc024c57dbd Mon Sep 17 00:00:00 2001 From: Ka-Hing Cheung Date: Wed, 11 Dec 2019 00:41:37 -0800 Subject: [PATCH] fix getxattr and listxattr (#72) 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. --- conversions.go | 15 ++------ samples/memfs/memfs.go | 11 +++--- samples/memfs/memfs_test.go | 68 +++++++++++++++++++++++-------------- 3 files changed, 51 insertions(+), 43 deletions(-) diff --git a/conversions.go b/conversions.go index 4e0cb35..fb034e9 100644 --- a/conversions.go +++ b/conversions.go @@ -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 { @@ -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) diff --git a/samples/memfs/memfs.go b/samples/memfs/memfs.go index 55a17bd..6cbb6bc 100644 --- a/samples/memfs/memfs.go +++ b/samples/memfs/memfs.go @@ -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 { @@ -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 { @@ -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 } @@ -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 } diff --git a/samples/memfs/memfs_test.go b/samples/memfs/memfs_test.go index ba6e59c..d2f0a1d 100644 --- a/samples/memfs/memfs_test.go +++ b/samples/memfs/memfs_test.go @@ -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) } @@ -1792,6 +1792,8 @@ 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") @@ -1799,53 +1801,69 @@ func (t *MemFSTest) NoXattrs() { 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() { @@ -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) }