From 14045511d3d91f36845a138359718c0fe7dff21c Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Mon, 2 Feb 2026 22:18:25 +0100 Subject: Use user-provided Listxattr buffer size This huge buffer showed up big time in ./profiling/ls.bash and caused a 2x LS benchmark slowdown. Instead of the fixed-size buffer, use the buffer size hint the user has provided us. We return inaccurate (larger than actual, which should be safe) numbers when the user has provided no buffer (size probe). --- fsck.go | 6 ++- internal/fusefrontend/node_xattr.go | 15 +++++++- internal/fusefrontend/node_xattr_darwin.go | 11 ++---- internal/fusefrontend/node_xattr_linux.go | 9 ++--- internal/fusefrontend_reverse/node_xattr.go | 11 +++++- internal/fusefrontend_reverse/node_xattr_darwin.go | 12 +++--- internal/fusefrontend_reverse/node_xattr_linux.go | 10 ++--- internal/nametransform/names_test.go | 12 ++++++ internal/syscallcompat/sys_common.go | 43 +--------------------- 9 files changed, 58 insertions(+), 71 deletions(-) diff --git a/fsck.go b/fsck.go index 2a76020..243eed9 100644 --- a/fsck.go +++ b/fsck.go @@ -12,6 +12,7 @@ import ( "syscall" "github.com/hanwen/go-fuse/v2/fuse" + "golang.org/x/sys/unix" "github.com/rfjakob/gocryptfs/v2/internal/exitcodes" "github.com/rfjakob/gocryptfs/v2/internal/fusefrontend" @@ -233,7 +234,10 @@ func (ck *fsckObj) watchMitigatedCorruptionsListXAttr(path string) { func (ck *fsckObj) xattrs(relPath string) { // Run ListXAttr() and catch transparently mitigated corruptions go ck.watchMitigatedCorruptionsListXAttr(relPath) - attrs, err := syscallcompat.Llistxattr(ck.abs(relPath)) + // TODO: smarter buffer sizing + buf := make([]byte, 64*1024) + sz, err := unix.Llistxattr(ck.abs(relPath), buf) + attrs := syscallcompat.ParseListxattrBlob(buf[:sz]) ck.watchDone <- struct{}{} if err != nil { fmt.Printf("fsck: error listing xattrs on %q: %v\n", relPath, err) diff --git a/internal/fusefrontend/node_xattr.go b/internal/fusefrontend/node_xattr.go index 1470a2a..2ca5ff7 100644 --- a/internal/fusefrontend/node_xattr.go +++ b/internal/fusefrontend/node_xattr.go @@ -9,6 +9,7 @@ import ( "github.com/hanwen/go-fuse/v2/fuse" + "github.com/rfjakob/gocryptfs/v2/internal/syscallcompat" "github.com/rfjakob/gocryptfs/v2/internal/tlog" ) @@ -136,10 +137,22 @@ func (n *Node) Listxattr(ctx context.Context, dest []byte) (uint32, syscall.Errn if rn.args.NoXattr { return 0, 0 } - cNames, errno := n.listXAttr() + // cBuf is the buffer for the encrypted attr names. Double the size of what the user + // requested to allow for ciphertext expansion due to padding and base64 encoding. + // TODO: double-check max expansion factor + cBuf := make([]byte, 2*len(dest)) + sz, errno := n.listXAttr(cBuf) if errno != 0 { return 0, errno } + // A call with empty dest is a size probe as described in man 2 llistxattr. + // Should return the size of the full attr list and success. + // Because we don't know the size of the decrypted names yet, we return the + // size of the encrypted names. This is slightly higher and hence safe. + if len(dest) == 0 { + return uint32(sz), 0 + } + cNames := syscallcompat.ParseListxattrBlob(cBuf[:sz]) var buf bytes.Buffer for _, curName := range cNames { // ACLs are passed through without encryption diff --git a/internal/fusefrontend/node_xattr_darwin.go b/internal/fusefrontend/node_xattr_darwin.go index f8f224f..ad0cd9e 100644 --- a/internal/fusefrontend/node_xattr_darwin.go +++ b/internal/fusefrontend/node_xattr_darwin.go @@ -88,7 +88,7 @@ func (n *Node) removeXAttr(cAttr string) (errno syscall.Errno) { return fs.ToErrno(err) } -func (n *Node) listXAttr() (out []string, errno syscall.Errno) { +func (n *Node) listXAttr(buf []byte) (sz int, errno syscall.Errno) { dirfd, cName, errno := n.prepareAtSyscallMyself() if errno != 0 { return @@ -98,13 +98,10 @@ func (n *Node) listXAttr() (out []string, errno syscall.Errno) { // O_NONBLOCK to not block on FIFOs. fd, err := syscallcompat.Openat(dirfd, cName, syscall.O_RDONLY|syscall.O_NONBLOCK|syscall.O_NOFOLLOW, 0) if err != nil { - return nil, fs.ToErrno(err) + return 0, fs.ToErrno(err) } defer syscall.Close(fd) - cNames, err := syscallcompat.Flistxattr(fd) - if err != nil { - return nil, fs.ToErrno(err) - } - return cNames, 0 + sz, err = unix.Flistxattr(fd, buf) + return sz, fs.ToErrno(err) } diff --git a/internal/fusefrontend/node_xattr_linux.go b/internal/fusefrontend/node_xattr_linux.go index 9964212..d1747e8 100644 --- a/internal/fusefrontend/node_xattr_linux.go +++ b/internal/fusefrontend/node_xattr_linux.go @@ -57,7 +57,7 @@ func (n *Node) removeXAttr(cAttr string) (errno syscall.Errno) { return fs.ToErrno(unix.Lremovexattr(procPath, cAttr)) } -func (n *Node) listXAttr() (out []string, errno syscall.Errno) { +func (n *Node) listXAttr(buf []byte) (sz int, errno syscall.Errno) { dirfd, cName, errno := n.prepareAtSyscallMyself() if errno != 0 { return @@ -65,9 +65,6 @@ func (n *Node) listXAttr() (out []string, errno syscall.Errno) { defer syscall.Close(dirfd) procPath := fmt.Sprintf("/proc/self/fd/%d/%s", dirfd, cName) - cNames, err := syscallcompat.Llistxattr(procPath) - if err != nil { - return nil, fs.ToErrno(err) - } - return cNames, 0 + sz, err := unix.Llistxattr(procPath, buf) + return sz, fs.ToErrno(err) } diff --git a/internal/fusefrontend_reverse/node_xattr.go b/internal/fusefrontend_reverse/node_xattr.go index f22764a..9e99cfa 100644 --- a/internal/fusefrontend_reverse/node_xattr.go +++ b/internal/fusefrontend_reverse/node_xattr.go @@ -7,6 +7,7 @@ import ( "syscall" "github.com/rfjakob/gocryptfs/v2/internal/pathiv" + "github.com/rfjakob/gocryptfs/v2/internal/syscallcompat" ) // We store encrypted xattrs under this prefix plus the base64-encoded @@ -65,10 +66,18 @@ func (n *Node) Listxattr(ctx context.Context, dest []byte) (uint32, syscall.Errn if rn.args.NoXattr { return 0, 0 } - pNames, errno := n.listXAttr() + // Can use dest as a temporary buffer + sz, errno := n.listXAttr(dest) if errno != 0 { return 0, errno } + // If dest empty, return the required size + if len(dest) == 0 { + // Asssume ciphertext expansion by a factor of 2 + // TODO: double-check max expansion factor + return uint32(sz * 2), 0 + } + pNames := syscallcompat.ParseListxattrBlob(dest[:sz]) var buf bytes.Buffer for _, pName := range pNames { // ACLs are passed through without encryption diff --git a/internal/fusefrontend_reverse/node_xattr_darwin.go b/internal/fusefrontend_reverse/node_xattr_darwin.go index 6816a18..1832f7f 100644 --- a/internal/fusefrontend_reverse/node_xattr_darwin.go +++ b/internal/fusefrontend_reverse/node_xattr_darwin.go @@ -4,6 +4,7 @@ import ( "syscall" "github.com/hanwen/go-fuse/v2/fs" + "golang.org/x/sys/unix" "github.com/rfjakob/gocryptfs/v2/internal/syscallcompat" ) @@ -33,7 +34,7 @@ func (n *Node) getXAttr(cAttr string) (out []byte, errno syscall.Errno) { return cData, 0 } -func (n *Node) listXAttr() (out []string, errno syscall.Errno) { +func (n *Node) listXAttr(buf []byte) (sz int, errno syscall.Errno) { d, errno := n.prepareAtSyscall("") if errno != 0 { return @@ -43,13 +44,10 @@ func (n *Node) listXAttr() (out []string, errno syscall.Errno) { // O_NONBLOCK to not block on FIFOs. fd, err := syscallcompat.Openat(d.dirfd, d.pName, syscall.O_RDONLY|syscall.O_NONBLOCK|syscall.O_NOFOLLOW, 0) if err != nil { - return nil, fs.ToErrno(err) + return 0, fs.ToErrno(err) } defer syscall.Close(fd) - pNames, err := syscallcompat.Flistxattr(fd) - if err != nil { - return nil, fs.ToErrno(err) - } - return pNames, 0 + sz, err = unix.Flistxattr(fd, buf) + return sz, fs.ToErrno(err) } diff --git a/internal/fusefrontend_reverse/node_xattr_linux.go b/internal/fusefrontend_reverse/node_xattr_linux.go index 3c574f5..226ace4 100644 --- a/internal/fusefrontend_reverse/node_xattr_linux.go +++ b/internal/fusefrontend_reverse/node_xattr_linux.go @@ -5,6 +5,7 @@ import ( "syscall" "github.com/hanwen/go-fuse/v2/fs" + "golang.org/x/sys/unix" "github.com/rfjakob/gocryptfs/v2/internal/syscallcompat" ) @@ -27,7 +28,7 @@ func (n *Node) getXAttr(cAttr string) (out []byte, errno syscall.Errno) { return pData, 0 } -func (n *Node) listXAttr() (out []string, errno syscall.Errno) { +func (n *Node) listXAttr(buf []byte) (sz int, errno syscall.Errno) { d, errno := n.prepareAtSyscall("") if errno != 0 { return @@ -35,9 +36,6 @@ func (n *Node) listXAttr() (out []string, errno syscall.Errno) { defer syscall.Close(d.dirfd) procPath := fmt.Sprintf("/proc/self/fd/%d/%s", d.dirfd, d.pName) - pNames, err := syscallcompat.Llistxattr(procPath) - if err != nil { - return nil, fs.ToErrno(err) - } - return pNames, 0 + sz, err := unix.Llistxattr(procPath, buf) + return sz, fs.ToErrno(err) } diff --git a/internal/nametransform/names_test.go b/internal/nametransform/names_test.go index 3c26c43..c016f19 100644 --- a/internal/nametransform/names_test.go +++ b/internal/nametransform/names_test.go @@ -98,3 +98,15 @@ func TestIsValidXattrName(t *testing.T) { } } } + +func TestNameCiphertextExpansion(t *testing.T) { + n := newLognamesTestInstance(NameMax) + for l := 1; l <= 300; l++ { + name := strings.Repeat("x", l) + cName, err := n.EncryptXattrName(name) + if err != nil { + t.Fatal(err) + } + t.Logf("Name length %d encrypted to %d bytes", l, len(cName)) + } +} diff --git a/internal/syscallcompat/sys_common.go b/internal/syscallcompat/sys_common.go index 1aa6a6e..1f08246 100644 --- a/internal/syscallcompat/sys_common.go +++ b/internal/syscallcompat/sys_common.go @@ -179,48 +179,7 @@ out: return val, nil } -// Flistxattr is a wrapper for unix.Flistxattr that handles buffer sizing and -// parsing the returned blob to a string slice. -func Flistxattr(fd int) (attrs []string, err error) { - // See the buffer sizing comments in getxattrSmartBuf. - // TODO: smarter buffer sizing? - buf := make([]byte, XATTR_BUFSZ) - sz, err := unix.Flistxattr(fd, buf) - if err == syscall.ERANGE { - // Do NOT return ERANGE - the user might retry ad inifinitum! - return nil, syscall.EOVERFLOW - } - if err != nil { - return nil, err - } - if sz >= XATTR_SIZE_MAX { - return nil, syscall.EOVERFLOW - } - attrs = parseListxattrBlob(buf[:sz]) - return attrs, nil -} - -// Llistxattr is a wrapper for unix.Llistxattr that handles buffer sizing and -// parsing the returned blob to a string slice. -func Llistxattr(path string) (attrs []string, err error) { - // TODO: smarter buffer sizing? - buf := make([]byte, XATTR_BUFSZ) - sz, err := unix.Llistxattr(path, buf) - if err == syscall.ERANGE { - // Do NOT return ERANGE - the user might retry ad inifinitum! - return nil, syscall.EOVERFLOW - } - if err != nil { - return nil, err - } - if sz >= XATTR_SIZE_MAX { - return nil, syscall.EOVERFLOW - } - attrs = parseListxattrBlob(buf[:sz]) - return attrs, nil -} - -func parseListxattrBlob(buf []byte) (attrs []string) { +func ParseListxattrBlob(buf []byte) (attrs []string) { parts := bytes.Split(buf, []byte{0}) for _, part := range parts { if len(part) == 0 { -- cgit v1.2.3