From e550bfbd052ddbec0ab214aa1991fad711cf013b Mon Sep 17 00:00:00 2001 From: Jing Chen Date: Wed, 6 Sep 2023 13:10:01 -0700 Subject: [PATCH] Have lgetxattr return ENODATA. It is a workaround to lgetxattr/lsetxattr working with security.capacity when running docker in gvisor. PiperOrigin-RevId: 563194162 --- pkg/sentry/fsimpl/gofer/gofer.go | 4 ++-- pkg/sentry/fsimpl/tmpfs/tmpfs.go | 2 ++ pkg/sentry/vfs/permissions.go | 5 +++++ test/syscalls/linux/xattr.cc | 15 +++++++++++++++ 4 files changed, 24 insertions(+), 2 deletions(-) diff --git a/pkg/sentry/fsimpl/gofer/gofer.go b/pkg/sentry/fsimpl/gofer/gofer.go index ca53adae7..c3163a73a 100644 --- a/pkg/sentry/fsimpl/gofer/gofer.go +++ b/pkg/sentry/fsimpl/gofer/gofer.go @@ -1399,7 +1399,7 @@ func (d *dentry) checkPermissions(creds *auth.Credentials, ats vfs.AccessTypes) } func (d *dentry) checkXattrPermissions(creds *auth.Credentials, name string, ats vfs.AccessTypes) error { - // Deny access to the "security" and "system" namespaces since applications + // Deny access to the "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, @@ -1407,7 +1407,7 @@ func (d *dentry) checkXattrPermissions(creds *auth.Credentials, name string, ats // // NOTE(b/202533394): Also disallow "trusted" namespace for now. This is // consistent with the VFS1 gofer client. - if strings.HasPrefix(name, linux.XATTR_SECURITY_PREFIX) || strings.HasPrefix(name, linux.XATTR_SYSTEM_PREFIX) || strings.HasPrefix(name, linux.XATTR_TRUSTED_PREFIX) { + if strings.HasPrefix(name, linux.XATTR_SYSTEM_PREFIX) || strings.HasPrefix(name, linux.XATTR_TRUSTED_PREFIX) { return linuxerr.EOPNOTSUPP } mode := linux.FileMode(d.mode.Load()) diff --git a/pkg/sentry/fsimpl/tmpfs/tmpfs.go b/pkg/sentry/fsimpl/tmpfs/tmpfs.go index c02a812f7..bc4635cb3 100644 --- a/pkg/sentry/fsimpl/tmpfs/tmpfs.go +++ b/pkg/sentry/fsimpl/tmpfs/tmpfs.go @@ -195,6 +195,8 @@ func (fstype FilesystemType) GetFilesystem(ctx context.Context, vfsObj *vfs.Virt allowXattrPrefix := map[string]struct{}{ linux.XATTR_TRUSTED_PREFIX: struct{}{}, linux.XATTR_USER_PREFIX: struct{}{}, + // The "security" namespace is allowed, but it always returns an error. + linux.XATTR_SECURITY_PREFIX: struct{}{}, } tmpfsOpts, tmpfsOptsOk := opts.InternalData.(FilesystemOpts) diff --git a/pkg/sentry/vfs/permissions.go b/pkg/sentry/vfs/permissions.go index 16e653a23..2ea5bae8a 100644 --- a/pkg/sentry/vfs/permissions.go +++ b/pkg/sentry/vfs/permissions.go @@ -323,6 +323,11 @@ func CheckXattrPermissions(creds *auth.Credentials, ats AccessTypes, mode linux. if filetype == linux.ModeDirectory && mode&linux.ModeSticky != 0 && ats.MayWrite() && !CanActAsOwner(creds, kuid) { return linuxerr.EPERM } + case strings.HasPrefix(name, linux.XATTR_SECURITY_PREFIX): + if ats.MayRead() { + return linuxerr.ENODATA + } + return linuxerr.EOPNOTSUPP } return nil } diff --git a/test/syscalls/linux/xattr.cc b/test/syscalls/linux/xattr.cc index 1b80f9721..065accb7e 100644 --- a/test/syscalls/linux/xattr.cc +++ b/test/syscalls/linux/xattr.cc @@ -105,6 +105,21 @@ TEST_F(XattrTest, XattrInvalidPrefix) { SyscallFailsWithErrno(EOPNOTSUPP)); } +TEST_F(XattrTest, SecurityCapacityXattr) { + SKIP_IF(!IsRunningOnGvisor()); + const char* path = test_file_name_.c_str(); + const char name[] = "security.capacity"; + const std::string val = ""; + EXPECT_THAT(lsetxattr(path, name, &val, val.size(), 0), + SyscallFailsWithErrno(EOPNOTSUPP)); + + int buf = 0; + EXPECT_THAT(lgetxattr(path, name, &buf, /*size=*/128), + SyscallFailsWithErrno(ENODATA)); + + EXPECT_THAT(lremovexattr(path, name), SyscallFailsWithErrno(EOPNOTSUPP)); +} + // Do not allow save/restore cycles after making the test file read-only, as // the restore will fail to open it with r/w permissions. TEST_F(XattrTest, XattrReadOnly) {