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

3 files changed, 47 insertions, 35 deletions

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.