summaryrefslogtreecommitdiffhomepage
path: root/pkg/sentry
diff options
context:
space:
mode:
authorAyush Ranjan <ayushranjan@google.com>2021-02-03 22:42:28 -0800
committergVisor bot <gvisor-bot@google.com>2021-02-03 22:44:51 -0800
commitf2c881f68498b542f21288559e3cb218673484f0 (patch)
tree32042a8f900ddb4d2b569859ab0f8a4405a7e5b6 /pkg/sentry
parent0dbc112979ff046e15a9616e98c4febc135ce77e (diff)
[vfs] Make sticky bit check consistent with Linux.
Our implementation of vfs.CheckDeleteSticky was not consistent with Linux, specifically not consistent with fs/linux.h:check_sticky(). One of the biggest differences was that the vfs implementation did not allow the owner of the sticky directory to delete files inside it that belonged to other users. This change makes our implementation consistent with Linux. Also adds an integration test to check for this. This bug is also present in VFS1. Updates #3027 PiperOrigin-RevId: 355557425
Diffstat (limited to 'pkg/sentry')
-rw-r--r--pkg/sentry/fsimpl/gofer/gofer.go8
-rw-r--r--pkg/sentry/fsimpl/overlay/filesystem.go6
-rw-r--r--pkg/sentry/fsimpl/overlay/overlay.go10
-rw-r--r--pkg/sentry/fsimpl/tmpfs/directory.go8
-rw-r--r--pkg/sentry/vfs/permissions.go6
5 files changed, 31 insertions, 7 deletions
diff --git a/pkg/sentry/fsimpl/gofer/gofer.go b/pkg/sentry/fsimpl/gofer/gofer.go
index 98f7bc52f..094d993a8 100644
--- a/pkg/sentry/fsimpl/gofer/gofer.go
+++ b/pkg/sentry/fsimpl/gofer/gofer.go
@@ -1216,7 +1216,13 @@ func (d *dentry) checkXattrPermissions(creds *auth.Credentials, name string, ats
}
func (d *dentry) mayDelete(creds *auth.Credentials, child *dentry) error {
- return vfs.CheckDeleteSticky(creds, linux.FileMode(atomic.LoadUint32(&d.mode)), auth.KUID(atomic.LoadUint32(&child.uid)))
+ return vfs.CheckDeleteSticky(
+ creds,
+ linux.FileMode(atomic.LoadUint32(&d.mode)),
+ auth.KUID(atomic.LoadUint32(&d.uid)),
+ auth.KUID(atomic.LoadUint32(&child.uid)),
+ auth.KGID(atomic.LoadUint32(&child.gid)),
+ )
}
func dentryUIDFromP9UID(uid p9.UID) uint32 {
diff --git a/pkg/sentry/fsimpl/overlay/filesystem.go b/pkg/sentry/fsimpl/overlay/filesystem.go
index e46f593c7..b36031291 100644
--- a/pkg/sentry/fsimpl/overlay/filesystem.go
+++ b/pkg/sentry/fsimpl/overlay/filesystem.go
@@ -1068,7 +1068,7 @@ func (fs *filesystem) RenameAt(ctx context.Context, rp *vfs.ResolvingPath, oldPa
if err != nil {
return err
}
- if err := vfs.CheckDeleteSticky(creds, linux.FileMode(atomic.LoadUint32(&oldParent.mode)), auth.KUID(atomic.LoadUint32(&renamed.uid))); err != nil {
+ if err := oldParent.mayDelete(creds, renamed); err != nil {
return err
}
if renamed.isDir() {
@@ -1317,7 +1317,7 @@ func (fs *filesystem) RmdirAt(ctx context.Context, rp *vfs.ResolvingPath) error
if !child.isDir() {
return syserror.ENOTDIR
}
- if err := vfs.CheckDeleteSticky(rp.Credentials(), linux.FileMode(atomic.LoadUint32(&parent.mode)), auth.KUID(atomic.LoadUint32(&child.uid))); err != nil {
+ if err := parent.mayDelete(rp.Credentials(), child); err != nil {
return err
}
child.dirMu.Lock()
@@ -1584,7 +1584,7 @@ func (fs *filesystem) UnlinkAt(ctx context.Context, rp *vfs.ResolvingPath) error
if child.isDir() {
return syserror.EISDIR
}
- if err := vfs.CheckDeleteSticky(rp.Credentials(), linux.FileMode(parentMode), auth.KUID(atomic.LoadUint32(&child.uid))); err != nil {
+ if err := parent.mayDelete(rp.Credentials(), child); err != nil {
return err
}
if err := vfsObj.PrepareDeleteDentry(mntns, &child.vfsd); err != nil {
diff --git a/pkg/sentry/fsimpl/overlay/overlay.go b/pkg/sentry/fsimpl/overlay/overlay.go
index 082fa6504..acd3684c6 100644
--- a/pkg/sentry/fsimpl/overlay/overlay.go
+++ b/pkg/sentry/fsimpl/overlay/overlay.go
@@ -760,6 +760,16 @@ func (d *dentry) updateAfterSetStatLocked(opts *vfs.SetStatOptions) {
}
}
+func (d *dentry) mayDelete(creds *auth.Credentials, child *dentry) error {
+ return vfs.CheckDeleteSticky(
+ creds,
+ linux.FileMode(atomic.LoadUint32(&d.mode)),
+ auth.KUID(atomic.LoadUint32(&d.uid)),
+ auth.KUID(atomic.LoadUint32(&child.uid)),
+ auth.KGID(atomic.LoadUint32(&child.gid)),
+ )
+}
+
// fileDescription is embedded by overlay implementations of
// vfs.FileDescriptionImpl.
//
diff --git a/pkg/sentry/fsimpl/tmpfs/directory.go b/pkg/sentry/fsimpl/tmpfs/directory.go
index e90669cf0..417ac2eff 100644
--- a/pkg/sentry/fsimpl/tmpfs/directory.go
+++ b/pkg/sentry/fsimpl/tmpfs/directory.go
@@ -84,7 +84,13 @@ func (dir *directory) removeChildLocked(child *dentry) {
}
func (dir *directory) mayDelete(creds *auth.Credentials, child *dentry) error {
- return vfs.CheckDeleteSticky(creds, linux.FileMode(atomic.LoadUint32(&dir.inode.mode)), auth.KUID(atomic.LoadUint32(&child.inode.uid)))
+ return vfs.CheckDeleteSticky(
+ creds,
+ linux.FileMode(atomic.LoadUint32(&dir.inode.mode)),
+ auth.KUID(atomic.LoadUint32(&dir.inode.uid)),
+ auth.KUID(atomic.LoadUint32(&child.inode.uid)),
+ auth.KGID(atomic.LoadUint32(&child.inode.gid)),
+ )
}
// +stateify savable
diff --git a/pkg/sentry/vfs/permissions.go b/pkg/sentry/vfs/permissions.go
index d48520d58..db6146fd2 100644
--- a/pkg/sentry/vfs/permissions.go
+++ b/pkg/sentry/vfs/permissions.go
@@ -243,11 +243,13 @@ func CheckSetStat(ctx context.Context, creds *auth.Credentials, opts *SetStatOpt
// the given file mode, and if so, checks whether creds has permission to
// remove a file owned by childKUID from a directory with the given mode.
// CheckDeleteSticky is consistent with fs/linux.h:check_sticky().
-func CheckDeleteSticky(creds *auth.Credentials, parentMode linux.FileMode, childKUID auth.KUID) error {
+func CheckDeleteSticky(creds *auth.Credentials, parentMode linux.FileMode, parentKUID auth.KUID, childKUID auth.KUID, childKGID auth.KGID) error {
if parentMode&linux.ModeSticky == 0 {
return nil
}
- if CanActAsOwner(creds, childKUID) {
+ if creds.EffectiveKUID == childKUID ||
+ creds.EffectiveKUID == parentKUID ||
+ HasCapabilityOnFile(creds, linux.CAP_FOWNER, childKUID, childKGID) {
return nil
}
return syserror.EPERM