Minor VFS2 xattr changes.

- Allow the gofer client to use most xattr namespaces. As documented by the
  updated comment, this is consistent with e.g. Linux's FUSE client, and allows
  gofers to provide extended attributes from FUSE filesystems.

- Make tmpfs' listxattr omit xattrs in the "trusted" namespace for
  non-privileged users.

PiperOrigin-RevId: 378778854

6 files changed, 83 insertions, 39 deletions

diff --git a/pkg/abi/linux/xattr.go b/pkg/abi/linux/xattr.go
index 8ef837f27..1fa7a4f4f 100644
--- a/pkg/abi/linux/xattr.go
+++ b/pkg/abi/linux/xattr.go
@@ -23,6 +23,12 @@ const (
 	XATTR_CREATE  = 1
 	XATTR_REPLACE = 2
 
+	XATTR_SECURITY_PREFIX     = "security."
+	XATTR_SECURITY_PREFIX_LEN = len(XATTR_SECURITY_PREFIX)
+
+	XATTR_SYSTEM_PREFIX     = "system."
+	XATTR_SYSTEM_PREFIX_LEN = len(XATTR_SYSTEM_PREFIX)
+
 	XATTR_TRUSTED_PREFIX     = "trusted."
 	XATTR_TRUSTED_PREFIX_LEN = len(XATTR_TRUSTED_PREFIX)
 
diff --git a/pkg/sentry/fsimpl/gofer/gofer.go b/pkg/sentry/fsimpl/gofer/gofer.go
index 21692d2ac..cf69e1b7a 100644
--- a/pkg/sentry/fsimpl/gofer/gofer.go
+++ b/pkg/sentry/fsimpl/gofer/gofer.go
@@ -1282,9 +1282,12 @@ func (d *dentry) checkPermissions(creds *auth.Credentials, ats vfs.AccessTypes)
 }
 
 func (d *dentry) checkXattrPermissions(creds *auth.Credentials, name string, ats vfs.AccessTypes) error {
-	// We only support xattrs prefixed with "user." (see b/148380782). Currently,
-	// there is no need to expose any other xattrs through a gofer.
-	if !strings.HasPrefix(name, linux.XATTR_USER_PREFIX) {
+	// Deny access to the "security" and "system" namespaces since applications
+	// may expect these to affect kernel behavior in unimplemented ways
+	// (b/148380782). Allow all other extended attributes to be passed through
+	// to the remote filesystem. This is inconsistent with Linux's 9p client,
+	// but consistent with other filesystems (e.g. FUSE).
+	if strings.HasPrefix(name, linux.XATTR_SECURITY_PREFIX) || strings.HasPrefix(name, linux.XATTR_SYSTEM_PREFIX) {
 		return syserror.EOPNOTSUPP
 	}
 	mode := linux.FileMode(atomic.LoadUint32(&d.mode))
@@ -1684,7 +1687,7 @@ func (d *dentry) setDeleted() {
 }
 
 func (d *dentry) listXattr(ctx context.Context, creds *auth.Credentials, size uint64) ([]string, error) {
-	if d.file.isNil() || !d.userXattrSupported() {
+	if d.file.isNil() {
 		return nil, nil
 	}
 	xattrMap, err := d.file.listXattr(ctx, size)
@@ -1693,10 +1696,7 @@ func (d *dentry) listXattr(ctx context.Context, creds *auth.Credentials, size ui
 	}
 	xattrs := make([]string, 0, len(xattrMap))
 	for x := range xattrMap {
-		// We only support xattrs in the user.* namespace.
-		if strings.HasPrefix(x, linux.XATTR_USER_PREFIX) {
-			xattrs = append(xattrs, x)
-		}
+		xattrs = append(xattrs, x)
 	}
 	return xattrs, nil
 }
@@ -1731,13 +1731,6 @@ func (d *dentry) removeXattr(ctx context.Context, creds *auth.Credentials, name
 	return d.file.removeXattr(ctx, name)
 }
 
-// Extended attributes in the user.* namespace are only supported for regular
-// files and directories.
-func (d *dentry) userXattrSupported() bool {
-	filetype := linux.FileMode(atomic.LoadUint32(&d.mode)).FileType()
-	return filetype == linux.ModeRegular || filetype == linux.ModeDirectory
-}
-
 // Preconditions:
 // * !d.isSynthetic().
 // * d.isRegularFile() || d.isDir().
diff --git a/pkg/sentry/fsimpl/tmpfs/filesystem.go b/pkg/sentry/fsimpl/tmpfs/filesystem.go
index ee7ff2961..f0f4297ef 100644
--- a/pkg/sentry/fsimpl/tmpfs/filesystem.go
+++ b/pkg/sentry/fsimpl/tmpfs/filesystem.go
@@ -822,7 +822,7 @@ func (fs *filesystem) ListXattrAt(ctx context.Context, rp *vfs.ResolvingPath, si
 	if err != nil {
 		return nil, err
 	}
-	return d.inode.listXattr(size)
+	return d.inode.listXattr(rp.Credentials(), size)
 }
 
 // GetXattrAt implements vfs.FilesystemImpl.GetXattrAt.
diff --git a/pkg/sentry/fsimpl/tmpfs/tmpfs.go b/pkg/sentry/fsimpl/tmpfs/tmpfs.go
index 9ae25ce9e..6b4367c42 100644
--- a/pkg/sentry/fsimpl/tmpfs/tmpfs.go
+++ b/pkg/sentry/fsimpl/tmpfs/tmpfs.go
@@ -717,44 +717,63 @@ func (i *inode) touchCMtimeLocked() {
 	atomic.StoreInt64(&i.ctime, now)
 }
 
-func (i *inode) listXattr(size uint64) ([]string, error) {
-	return i.xattrs.ListXattr(size)
+func checkXattrName(name string) error {
+	// Linux's tmpfs supports "security" and "trusted" xattr namespaces, and
+	// (depending on build configuration) POSIX ACL xattr namespaces
+	// ("system.posix_acl_access" and "system.posix_acl_default"). We don't
+	// support POSIX ACLs or the "security" namespace (b/148380782).
+	if strings.HasPrefix(name, linux.XATTR_TRUSTED_PREFIX) {
+		return nil
+	}
+	// We support the "user" namespace because we have tests that depend on
+	// this feature.
+	if strings.HasPrefix(name, linux.XATTR_USER_PREFIX) {
+		return nil
+	}
+	return syserror.EOPNOTSUPP
+}
+
+func (i *inode) listXattr(creds *auth.Credentials, size uint64) ([]string, error) {
+	return i.xattrs.ListXattr(creds, size)
 }
 
 func (i *inode) getXattr(creds *auth.Credentials, opts *vfs.GetXattrOptions) (string, error) {
-	if err := i.checkXattrPermissions(creds, opts.Name, vfs.MayRead); err != nil {
+	if err := checkXattrName(opts.Name); err != nil {
 		return "", err
 	}
-	return i.xattrs.GetXattr(opts)
+	mode := linux.FileMode(atomic.LoadUint32(&i.mode))
+	kuid := auth.KUID(atomic.LoadUint32(&i.uid))
+	kgid := auth.KGID(atomic.LoadUint32(&i.gid))
+	if err := vfs.GenericCheckPermissions(creds, vfs.MayRead, mode, kuid, kgid); err != nil {
+		return "", err
+	}
+	return i.xattrs.GetXattr(creds, mode, kuid, opts)
 }
 
 func (i *inode) setXattr(creds *auth.Credentials, opts *vfs.SetXattrOptions) error {
-	if err := i.checkXattrPermissions(creds, opts.Name, vfs.MayWrite); err != nil {
+	if err := checkXattrName(opts.Name); err != nil {
 		return err
 	}
-	return i.xattrs.SetXattr(opts)
-}
-
-func (i *inode) removeXattr(creds *auth.Credentials, name string) error {
-	if err := i.checkXattrPermissions(creds, name, vfs.MayWrite); err != nil {
+	mode := linux.FileMode(atomic.LoadUint32(&i.mode))
+	kuid := auth.KUID(atomic.LoadUint32(&i.uid))
+	kgid := auth.KGID(atomic.LoadUint32(&i.gid))
+	if err := vfs.GenericCheckPermissions(creds, vfs.MayWrite, mode, kuid, kgid); err != nil {
 		return err
 	}
-	return i.xattrs.RemoveXattr(name)
+	return i.xattrs.SetXattr(creds, mode, kuid, opts)
 }
 
-func (i *inode) checkXattrPermissions(creds *auth.Credentials, name string, ats vfs.AccessTypes) error {
-	// We currently only support extended attributes in the user.* and
-	// trusted.* namespaces. See b/148380782.
-	if !strings.HasPrefix(name, linux.XATTR_USER_PREFIX) && !strings.HasPrefix(name, linux.XATTR_TRUSTED_PREFIX) {
-		return syserror.EOPNOTSUPP
+func (i *inode) removeXattr(creds *auth.Credentials, name string) error {
+	if err := checkXattrName(name); err != nil {
+		return err
 	}
 	mode := linux.FileMode(atomic.LoadUint32(&i.mode))
 	kuid := auth.KUID(atomic.LoadUint32(&i.uid))
 	kgid := auth.KGID(atomic.LoadUint32(&i.gid))
-	if err := vfs.GenericCheckPermissions(creds, ats, mode, kuid, kgid); err != nil {
+	if err := vfs.GenericCheckPermissions(creds, vfs.MayWrite, mode, kuid, kgid); err != nil {
 		return err
 	}
-	return vfs.CheckXattrPermissions(creds, ats, mode, kuid, name)
+	return i.xattrs.RemoveXattr(creds, mode, kuid, name)
 }
 
 // fileDescription is embedded by tmpfs implementations of
@@ -807,7 +826,7 @@ func (fd *fileDescription) StatFS(ctx context.Context) (linux.Statfs, error) {
 
 // ListXattr implements vfs.FileDescriptionImpl.ListXattr.
 func (fd *fileDescription) ListXattr(ctx context.Context, size uint64) ([]string, error) {
-	return fd.inode().listXattr(size)
+	return fd.inode().listXattr(auth.CredentialsFromContext(ctx), size)
 }
 
 // GetXattr implements vfs.FileDescriptionImpl.GetXattr.
diff --git a/pkg/sentry/vfs/memxattr/BUILD b/pkg/sentry/vfs/memxattr/BUILD
index d8c4d27b9..ea82f4987 100644
--- a/pkg/sentry/vfs/memxattr/BUILD
+++ b/pkg/sentry/vfs/memxattr/BUILD
@@ -8,6 +8,7 @@ go_library(
     visibility = ["//pkg/sentry:internal"],
     deps = [
         "//pkg/abi/linux",
+        "//pkg/sentry/kernel/auth",
         "//pkg/sentry/vfs",
         "//pkg/sync",
         "//pkg/syserror",
diff --git a/pkg/sentry/vfs/memxattr/xattr.go b/pkg/sentry/vfs/memxattr/xattr.go
index 638b5d830..9b7953fa3 100644
--- a/pkg/sentry/vfs/memxattr/xattr.go
+++ b/pkg/sentry/vfs/memxattr/xattr.go
@@ -17,7 +17,10 @@
 package memxattr
 
 import (
+	"strings"
+
 	"gvisor.dev/gvisor/pkg/abi/linux"
+	"gvisor.dev/gvisor/pkg/sentry/kernel/auth"
 	"gvisor.dev/gvisor/pkg/sentry/vfs"
 	"gvisor.dev/gvisor/pkg/sync"
 	"gvisor.dev/gvisor/pkg/syserror"
@@ -26,6 +29,9 @@ import (
 // SimpleExtendedAttributes implements extended attributes using a map of
 // names to values.
 //
+// SimpleExtendedAttributes calls vfs.CheckXattrPermissions, so callers are not
+// required to do so.
+//
 // +stateify savable
 type SimpleExtendedAttributes struct {
 	// mu protects the below fields.
@@ -34,7 +40,11 @@ type SimpleExtendedAttributes struct {
 }
 
 // GetXattr returns the value at 'name'.
-func (x *SimpleExtendedAttributes) GetXattr(opts *vfs.GetXattrOptions) (string, error) {
+func (x *SimpleExtendedAttributes) GetXattr(creds *auth.Credentials, mode linux.FileMode, kuid auth.KUID, opts *vfs.GetXattrOptions) (string, error) {
+	if err := vfs.CheckXattrPermissions(creds, vfs.MayRead, mode, kuid, opts.Name); err != nil {
+		return "", err
+	}
+
 	x.mu.RLock()
 	value, ok := x.xattrs[opts.Name]
 	x.mu.RUnlock()
@@ -50,7 +60,11 @@ func (x *SimpleExtendedAttributes) GetXattr(opts *vfs.GetXattrOptions) (string,
 }
 
 // SetXattr sets 'value' at 'name'.
-func (x *SimpleExtendedAttributes) SetXattr(opts *vfs.SetXattrOptions) error {
+func (x *SimpleExtendedAttributes) SetXattr(creds *auth.Credentials, mode linux.FileMode, kuid auth.KUID, opts *vfs.SetXattrOptions) error {
+	if err := vfs.CheckXattrPermissions(creds, vfs.MayWrite, mode, kuid, opts.Name); err != nil {
+		return err
+	}
+
 	x.mu.Lock()
 	defer x.mu.Unlock()
 	if x.xattrs == nil {
@@ -73,12 +87,19 @@ func (x *SimpleExtendedAttributes) SetXattr(opts *vfs.SetXattrOptions) error {
 }
 
 // ListXattr returns all names in xattrs.
-func (x *SimpleExtendedAttributes) ListXattr(size uint64) ([]string, error) {
+func (x *SimpleExtendedAttributes) ListXattr(creds *auth.Credentials, size uint64) ([]string, error) {
 	// Keep track of the size of the buffer needed in listxattr(2) for the list.
 	listSize := 0
 	x.mu.RLock()
 	names := make([]string, 0, len(x.xattrs))
+	haveCap := creds.HasCapability(linux.CAP_SYS_ADMIN)
 	for n := range x.xattrs {
+		// Hide extended attributes in the "trusted" namespace from
+		// non-privileged users. This is consistent with Linux's
+		// fs/xattr.c:simple_xattr_list().
+		if !haveCap && strings.HasPrefix(n, linux.XATTR_TRUSTED_PREFIX) {
+			continue
+		}
 		names = append(names, n)
 		// Add one byte per null terminator.
 		listSize += len(n) + 1
@@ -91,7 +112,11 @@ func (x *SimpleExtendedAttributes) ListXattr(size uint64) ([]string, error) {
 }
 
 // RemoveXattr removes the xattr at 'name'.
-func (x *SimpleExtendedAttributes) RemoveXattr(name string) error {
+func (x *SimpleExtendedAttributes) RemoveXattr(creds *auth.Credentials, mode linux.FileMode, kuid auth.KUID, name string) error {
+	if err := vfs.CheckXattrPermissions(creds, vfs.MayWrite, mode, kuid, name); err != nil {
+		return err
+	}
+
 	x.mu.Lock()
 	defer x.mu.Unlock()
 	if _, ok := x.xattrs[name]; !ok {