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.