summaryrefslogtreecommitdiffhomepage
path: root/pkg
diff options
context:
space:
mode:
authorJamie Liu <jamieliu@google.com>2020-05-04 18:00:56 -0700
committergVisor bot <gvisor-bot@google.com>2020-05-05 09:19:52 -0700
commit35951c3671f3d429399eb581ad9da3b56e2a5f5a (patch)
tree292f00eb8943be7cdf261bfeb75bd9b517b96d69 /pkg
parentda71dc7fddda387232b243c6176de21a1208ad0c (diff)
Translate p9.NoUID/GID to OverflowUID/GID.
p9.NoUID/GID (== uint32(-1) == auth.NoID) is not a valid auth.KUID/KGID; in particular, using it for file ownership causes capabilities to be ineffective since file capabilities require that the file's KUID and KGID are mapped into the capability holder's user namespace [1], and auth.NoID is not mapped into any user namespace. Map p9.NoUID/GID to a different, valid KUID/KGID; in the unlikely case that an application actually using the overflow KUID/KGID attempts an operation that is consequently permitted by client permission checks, the remote operation will still fail with EPERM. Since this changes the VFS2 gofer client to no longer ignore the invalid IDs entirely, this CL both permits and requires that we change synthetic mount point creation to use root credentials. [1] See fs.Inode.CheckCapability or vfs.GenericCheckPermissions. PiperOrigin-RevId: 309856455
Diffstat (limited to 'pkg')
-rw-r--r--pkg/sentry/fs/gofer/attr.go12
-rw-r--r--pkg/sentry/fsimpl/gofer/gofer.go26
2 files changed, 30 insertions, 8 deletions
diff --git a/pkg/sentry/fs/gofer/attr.go b/pkg/sentry/fs/gofer/attr.go
index 6db4b762d..d481baf77 100644
--- a/pkg/sentry/fs/gofer/attr.go
+++ b/pkg/sentry/fs/gofer/attr.go
@@ -75,10 +75,18 @@ func owner(mounter fs.FileOwner, valid p9.AttrMask, pattr p9.Attr) fs.FileOwner
// task's EUID/EGID.
owner := mounter
if valid.UID {
- owner.UID = auth.KUID(pattr.UID)
+ if pattr.UID.Ok() {
+ owner.UID = auth.KUID(pattr.UID)
+ } else {
+ owner.UID = auth.KUID(auth.OverflowUID)
+ }
}
if valid.GID {
- owner.GID = auth.KGID(pattr.GID)
+ if pattr.GID.Ok() {
+ owner.GID = auth.KGID(pattr.GID)
+ } else {
+ owner.GID = auth.KGID(auth.OverflowGID)
+ }
}
return owner
}
diff --git a/pkg/sentry/fsimpl/gofer/gofer.go b/pkg/sentry/fsimpl/gofer/gofer.go
index 2881c7bdd..1d9caf127 100644
--- a/pkg/sentry/fsimpl/gofer/gofer.go
+++ b/pkg/sentry/fsimpl/gofer/gofer.go
@@ -663,11 +663,11 @@ func (fs *filesystem) newDentry(ctx context.Context, file p9file, qid p9.QID, ma
},
}
d.pf.dentry = d
- if mask.UID && attr.UID != auth.NoID {
- d.uid = uint32(attr.UID)
+ if mask.UID {
+ d.uid = dentryUIDFromP9UID(attr.UID)
}
- if mask.GID && attr.GID != auth.NoID {
- d.gid = uint32(attr.GID)
+ if mask.GID {
+ d.gid = dentryGIDFromP9GID(attr.GID)
}
if mask.Size {
d.size = attr.Size
@@ -718,10 +718,10 @@ func (d *dentry) updateFromP9Attrs(mask p9.AttrMask, attr *p9.Attr) {
atomic.StoreUint32(&d.mode, uint32(attr.Mode))
}
if mask.UID {
- atomic.StoreUint32(&d.uid, uint32(attr.UID))
+ atomic.StoreUint32(&d.uid, dentryUIDFromP9UID(attr.UID))
}
if mask.GID {
- atomic.StoreUint32(&d.gid, uint32(attr.GID))
+ atomic.StoreUint32(&d.gid, dentryGIDFromP9GID(attr.GID))
}
// There is no P9_GETATTR_* bit for I/O block size.
if attr.BlockSize != 0 {
@@ -939,6 +939,20 @@ func (d *dentry) checkPermissions(creds *auth.Credentials, ats vfs.AccessTypes)
return vfs.GenericCheckPermissions(creds, ats, linux.FileMode(atomic.LoadUint32(&d.mode)), auth.KUID(atomic.LoadUint32(&d.uid)), auth.KGID(atomic.LoadUint32(&d.gid)))
}
+func dentryUIDFromP9UID(uid p9.UID) uint32 {
+ if !uid.Ok() {
+ return uint32(auth.OverflowUID)
+ }
+ return uint32(uid)
+}
+
+func dentryGIDFromP9GID(gid p9.GID) uint32 {
+ if !gid.Ok() {
+ return uint32(auth.OverflowGID)
+ }
+ return uint32(gid)
+}
+
// IncRef implements vfs.DentryImpl.IncRef.
func (d *dentry) IncRef() {
// d.refs may be 0 if d.fs.renameMu is locked, which serializes against