sentry/kernel: don't use Task.mu to protect Task.creds

Task.creds is documented as being "owned by the task goroutine", i.e. it can
only be mutated by the task goroutine, precluding the need for writer
synchronization. (AFAIK, no current Linux feature permits one task to change
another's credentials; capset(2) did before the introduction of VFS
capabilities, but gVisor doesn't support this. Libcs implement functions like
setresuid() by signaling every thread to execute the setresuid() syscall to
change their own credentials.) Prior to cl/185803179 and cl/205315612, the
mutex was needed to avoid data races on auth.Credentials fields; then prior to
cl/254989492, the mutex was needed to avoid data races on the auth.Credentials
pointer; now the mutex is not needed at all.

PiperOrigin-RevId: 777692758

Author: nixprime

pkg/sentry/kernel/task.go

@@ -454,10 +454,9 @@ type Task struct {
 
 	// creds is the task's credentials.
 	//
-	// creds.Load() may be called without synchronization. creds.Store() is
-	// serialized by mu. creds is owned by the task goroutine. All
-	// auth.Credentials objects that creds may point to, or have pointed to
-	// in the past, must be treated as immutable.
+	// creds is owned by the task goroutine. All auth.Credentials objects that
+	// creds may point to, or have pointed to in the past, must be treated as
+	// immutable.
 	creds auth.AtomicPtrCredentials
 
 	// utsns is the task's UTS namespace.

pkg/sentry/kernel/task_exec.go

@@ -245,10 +245,10 @@ func (r *runSyscallAfterExecStop) execute(t *Task) taskRunState {
 
 	// Switch to the new process.
 	t.MemoryManager().Deactivate()
-	t.mu.Lock()
 	// Update credentials to reflect the execve. This should precede switching
 	// MMs to ensure that dumpability has been reset first, if needed.
-	t.updateCredsForExecLocked()
+	t.updateCredsForExec()
+	t.mu.Lock()
 	oldImage := t.image
 	t.image = *r.image
 	t.mu.Unlock()

pkg/sentry/kernel/task_identity.go

@@ -44,15 +44,14 @@ func (t *Task) HasCapability(cp linux.Capability) bool {
 }
 
 // SetUID implements the semantics of setuid(2).
+//
+// Preconditions: The caller must be running on the task goroutine.
 func (t *Task) SetUID(uid auth.UID) error {
 	// setuid considers -1 to be invalid.
 	if !uid.Ok() {
 		return linuxerr.EINVAL
 	}
 
-	t.mu.Lock()
-	defer t.mu.Unlock()
-
 	creds := t.Credentials()
 	kuid := creds.UserNamespace.MapToKUID(uid)
 	if !kuid.Ok() {
@@ -63,7 +62,7 @@ func (t *Task) SetUID(uid auth.UID) error {
 	// the CAP_SETUID capability), the real UID and saved set-user-ID are also
 	// set." - setuid(2)
 	if creds.HasCapability(linux.CAP_SETUID) {
-		t.setKUIDsUncheckedLocked(kuid, kuid, kuid)
+		t.setKUIDsUnchecked(kuid, kuid, kuid)
 		return nil
 	}
 	// "EPERM: The user is not privileged (Linux: does not have the CAP_SETUID
@@ -72,14 +71,14 @@ func (t *Task) SetUID(uid auth.UID) error {
 	if kuid != creds.RealKUID && kuid != creds.SavedKUID {
 		return linuxerr.EPERM
 	}
-	t.setKUIDsUncheckedLocked(creds.RealKUID, kuid, creds.SavedKUID)
+	t.setKUIDsUnchecked(creds.RealKUID, kuid, creds.SavedKUID)
 	return nil
 }
 
 // SetREUID implements the semantics of setreuid(2).
+//
+// Preconditions: The caller must be running on the task goroutine.
 func (t *Task) SetREUID(r, e auth.UID) error {
-	t.mu.Lock()
-	defer t.mu.Unlock()
 	// "Supplying a value of -1 for either the real or effective user ID forces
 	// the system to leave that ID unchanged." - setreuid(2)
 	creds := t.Credentials()
@@ -116,14 +115,14 @@ func (t *Task) SetREUID(r, e auth.UID) error {
 	if r.Ok() || (e.Ok() && newE != creds.EffectiveKUID) {
 		newS = newE
 	}
-	t.setKUIDsUncheckedLocked(newR, newE, newS)
+	t.setKUIDsUnchecked(newR, newE, newS)
 	return nil
 }
 
 // SetRESUID implements the semantics of the setresuid(2) syscall.
+//
+// Preconditions: The caller must be running on the task goroutine.
 func (t *Task) SetRESUID(r, e, s auth.UID) error {
-	t.mu.Lock()
-	defer t.mu.Unlock()
 	// "Unprivileged user processes may change the real UID, effective UID, and
 	// saved set-user-ID, each to one of: the current real UID, the current
 	// effective UID or the current saved set-user-ID. Privileged processes (on
@@ -154,12 +153,12 @@ func (t *Task) SetRESUID(r, e, s auth.UID) error {
 			return err
 		}
 	}
-	t.setKUIDsUncheckedLocked(newR, newE, newS)
+	t.setKUIDsUnchecked(newR, newE, newS)
 	return nil
 }
 
-// Preconditions: t.mu must be locked.
-func (t *Task) setKUIDsUncheckedLocked(newR, newE, newS auth.KUID) {
+// Preconditions: The caller must be running on the task goroutine.
+func (t *Task) setKUIDsUnchecked(newR, newE, newS auth.KUID) {
 	creds := t.Credentials().Fork() // The credentials object is immutable. See doc for creds.
 	root := creds.UserNamespace.MapToKUID(auth.RootUID)
 	oldR, oldE, oldS := creds.RealKUID, creds.EffectiveKUID, creds.SavedKUID
@@ -221,35 +220,33 @@ func (t *Task) setKUIDsUncheckedLocked(newR, newE, newS auth.KUID) {
 }
 
 // SetGID implements the semantics of setgid(2).
+//
+// Preconditions: The caller must be running on the task goroutine.
 func (t *Task) SetGID(gid auth.GID) error {
 	if !gid.Ok() {
 		return linuxerr.EINVAL
 	}
 
-	t.mu.Lock()
-	defer t.mu.Unlock()
-
 	creds := t.Credentials()
 	kgid := creds.UserNamespace.MapToKGID(gid)
 	if !kgid.Ok() {
 		return linuxerr.EINVAL
 	}
 	if creds.HasCapability(linux.CAP_SETGID) {
-		t.setKGIDsUncheckedLocked(kgid, kgid, kgid)
+		t.setKGIDsUnchecked(kgid, kgid, kgid)
 		return nil
 	}
 	if kgid != creds.RealKGID && kgid != creds.SavedKGID {
 		return linuxerr.EPERM
 	}
-	t.setKGIDsUncheckedLocked(creds.RealKGID, kgid, creds.SavedKGID)
+	t.setKGIDsUnchecked(creds.RealKGID, kgid, creds.SavedKGID)
 	return nil
 }
 
 // SetREGID implements the semantics of setregid(2).
+//
+// Preconditions: The caller must be running on the task goroutine.
 func (t *Task) SetREGID(r, e auth.GID) error {
-	t.mu.Lock()
-	defer t.mu.Unlock()
-
 	creds := t.Credentials()
 	newR := creds.RealKGID
 	if r.Ok() {
@@ -277,17 +274,16 @@ func (t *Task) SetREGID(r, e auth.GID) error {
 	if r.Ok() || (e.Ok() && newE != creds.EffectiveKGID) {
 		newS = newE
 	}
-	t.setKGIDsUncheckedLocked(newR, newE, newS)
+	t.setKGIDsUnchecked(newR, newE, newS)
 	return nil
 }
 
 // SetRESGID implements the semantics of the setresgid(2) syscall.
+//
+// Preconditions: The caller must be running on the task goroutine.
 func (t *Task) SetRESGID(r, e, s auth.GID) error {
 	var err error
 
-	t.mu.Lock()
-	defer t.mu.Unlock()
-
 	creds := t.Credentials()
 	newR := creds.RealKGID
 	if r.Ok() {
@@ -310,11 +306,12 @@ func (t *Task) SetRESGID(r, e, s auth.GID) error {
 			return err
 		}
 	}
-	t.setKGIDsUncheckedLocked(newR, newE, newS)
+	t.setKGIDsUnchecked(newR, newE, newS)
 	return nil
 }
 
-func (t *Task) setKGIDsUncheckedLocked(newR, newE, newS auth.KGID) {
+// Preconditions: The caller must be running on the task goroutine.
+func (t *Task) setKGIDsUnchecked(newR, newE, newS auth.KGID) {
 	creds := t.Credentials().Fork() // The credentials object is immutable. See doc for creds.
 	oldE := creds.EffectiveKGID
 	creds.RealKGID, creds.EffectiveKGID, creds.SavedKGID = newR, newE, newS
@@ -338,6 +335,8 @@ func (t *Task) setKGIDsUncheckedLocked(newR, newE, newS auth.KGID) {
 
 // SetExtraGIDs attempts to change t's supplemental groups. All IDs are
 // interpreted as being in t's user namespace.
+//
+// Preconditions: The caller must be running on the task goroutine.
 func (t *Task) SetExtraGIDs(gids []auth.GID) error {
 	t.mu.Lock()
 	defer t.mu.Unlock()
@@ -364,9 +363,9 @@ var weakCaps = auth.CapabilitySetOf(linux.CAP_NET_RAW)
 
 // SetCapabilitySets attempts to change t's permitted, inheritable, and
 // effective capability sets.
+//
+// Preconditions: The caller must be running on the task goroutine.
 func (t *Task) SetCapabilitySets(permitted, inheritable, effective auth.CapabilitySet) error {
-	t.mu.Lock()
-	defer t.mu.Unlock()
 	// "Permitted: This is a limiting superset for the effective capabilities
 	// that the thread may assume." - capabilities(7)
 	if effective & ^permitted != 0 {
@@ -407,9 +406,9 @@ func (t *Task) SetCapabilitySets(permitted, inheritable, effective auth.Capabili
 
 // DropBoundingCapability attempts to drop capability cp from t's capability
 // bounding set.
+//
+// Preconditions: The caller must be running on the task goroutine.
 func (t *Task) DropBoundingCapability(cp linux.Capability) error {
-	t.mu.Lock()
-	defer t.mu.Unlock()
 	creds := t.Credentials()
 	if !creds.HasCapability(linux.CAP_SETPCAP) {
 		return linuxerr.EPERM
@@ -421,10 +420,9 @@ func (t *Task) DropBoundingCapability(cp linux.Capability) error {
 }
 
 // SetUserNamespace attempts to move c into ns.
+//
+// Preconditions: The caller must be running on the task goroutine.
 func (t *Task) SetUserNamespace(ns *auth.UserNamespace) error {
-	t.mu.Lock()
-	defer t.mu.Unlock()
-
 	creds := t.Credentials()
 	// "A process reassociating itself with a user namespace must have the
 	// CAP_SYS_ADMIN capability in the target user namespace." - setns(2)
@@ -457,6 +455,8 @@ func (t *Task) SetUserNamespace(ns *auth.UserNamespace) error {
 }
 
 // SetKeepCaps will set the keep capabilities flag PR_SET_KEEPCAPS.
+//
+// Preconditions: The caller must be running on the task goroutine.
 func (t *Task) SetKeepCaps(k bool) {
 	t.mu.Lock()
 	defer t.mu.Unlock()
@@ -465,7 +465,7 @@ func (t *Task) SetKeepCaps(k bool) {
 	t.creds.Store(creds)
 }
 
-// updateCredsForExecLocked updates t.creds to reflect an execve().
+// updateCredsForExec updates t.creds to reflect an execve().
 //
 // NOTE(b/30815691): We currently do not implement privileged executables
 // (set-user/group-ID bits and file capabilities). This allows us to make a lot
@@ -485,9 +485,7 @@ func (t *Task) SetKeepCaps(k bool) {
 //   - Task.ptraceAttach does not serialize with execve as it does in Linux,
 //     since no_new_privs being set has the same effect as the presence of an
 //     unprivileged tracer.
-//
-// Preconditions: t.mu must be locked.
-func (t *Task) updateCredsForExecLocked() {
+func (t *Task) updateCredsForExec() {
 	// """
 	// During an execve(2), the kernel calculates the new capabilities of
 	// the process using the following algorithm:

pkg/sentry/loader/loader.go

@@ -321,7 +321,7 @@ func Load(ctx context.Context, args LoadArgs, extraAuxv []arch.AuxEntry, vdso *V
 		arch.AuxEntry{linux.AT_GID, hostarch.Addr(c.RealKGID.In(c.UserNamespace).OrOverflow())},
 		arch.AuxEntry{linux.AT_EGID, hostarch.Addr(c.EffectiveKGID.In(c.UserNamespace).OrOverflow())},
 		// The conditions that require AT_SECURE = 1 never arise. See
-		// kernel.Task.updateCredsForExecLocked.
+		// kernel.Task.updateCredsForExec.
 		arch.AuxEntry{linux.AT_SECURE, 0},
 		arch.AuxEntry{linux.AT_CLKTCK, linux.CLOCKS_PER_SEC},
 		arch.AuxEntry{linux.AT_EXECFN, execfn},

pkg/sentry/syscalls/linux/sys_prctl.go

@@ -166,7 +166,7 @@ func Prctl(t *kernel.Task, sysno uintptr, args arch.SyscallArguments) (uintptr,
 			return 0, nil, linuxerr.EINVAL
 		}
 		// PR_SET_NO_NEW_PRIVS is assumed to always be set.
-		// See kernel.Task.updateCredsForExecLocked.
+		// See kernel.Task.updateCredsForExec.
 		return 0, nil, nil
 
 	case linux.PR_GET_NO_NEW_PRIVS: