kernel: Do not use gSchedulerLock when accesing UID and GID

Reads and writes to uid_t and gid_t are atomic anyway. The only real
problem that may happen here is inconsistent state of triples
effective_{u, g}id, saved_set_{u, g}id, real_{u, g}id, but team locks
protect us against that.
This commit is contained in:
Pawel Dziepak
2013-10-30 02:57:45 +01:00
parent 9c0ff0eed1
commit d54a9e0a41
2 changed files with 7 additions and 34 deletions
+4 -11
View File
@@ -315,7 +315,6 @@ Signal::SetTo(uint32 number)
fErrorCode = 0; fErrorCode = 0;
fSendingProcess = team->id; fSendingProcess = team->id;
fSendingUser = team->effective_uid; fSendingUser = team->effective_uid;
// assuming scheduler lock is being held
fStatus = 0; fStatus = 0;
fPollBand = 0; fPollBand = 0;
fAddress = NULL; fAddress = NULL;
@@ -1322,19 +1321,13 @@ has_signals_pending(Thread* thread)
/*! Checks whether the current user has permission to send a signal to the given /*! Checks whether the current user has permission to send a signal to the given
target team. target team.
The caller must hold the scheduler lock or \a team's lock.
\param team The target team. \param team The target team.
\param schedulerLocked \c true, if the caller holds the scheduler lock,
\c false otherwise.
*/ */
static bool static bool
has_permission_to_signal(Team* team, bool schedulerLocked) has_permission_to_signal(Signal* signal, Team* team)
{ {
// get the current user // get the current user
uid_t currentUser = schedulerLocked uid_t currentUser = signal->SendingUser();
? thread_get_current_thread()->team->effective_uid
: geteuid();
// root is omnipotent -- in the other cases the current user must match the // root is omnipotent -- in the other cases the current user must match the
// target team's // target team's
@@ -1372,7 +1365,7 @@ send_signal_to_thread_locked(Thread* thread, uint32 signalNumber,
BReference<Signal> signalReference(signal, true); BReference<Signal> signalReference(signal, true);
if ((flags & B_CHECK_PERMISSION) != 0) { if ((flags & B_CHECK_PERMISSION) != 0) {
if (!has_permission_to_signal(thread->team, true)) if (!has_permission_to_signal(signal, thread->team))
return EPERM; return EPERM;
} }
@@ -1566,7 +1559,7 @@ send_signal_to_team_locked(Team* team, uint32 signalNumber, Signal* signal,
BReference<Signal> signalReference(signal, true); BReference<Signal> signalReference(signal, true);
if ((flags & B_CHECK_PERMISSION) != 0) { if ((flags & B_CHECK_PERMISSION) != 0) {
if (!has_permission_to_signal(team, true)) if (!has_permission_to_signal(signal, team))
return EPERM; return EPERM;
} }
+3 -23
View File
@@ -54,7 +54,6 @@ common_setregid(gid_t rgid, gid_t egid, bool setAllIfPrivileged, bool kernel)
// setgid() semantics: If privileged set both, real, effective and // setgid() semantics: If privileged set both, real, effective and
// saved set-gid, otherwise set the effective gid. // saved set-gid, otherwise set the effective gid.
if (privileged) { if (privileged) {
InterruptsSpinLocker schedulerLocker(gSchedulerLock);
team->saved_set_gid = rgid; team->saved_set_gid = rgid;
team->real_gid = rgid; team->real_gid = rgid;
team->effective_gid = rgid; team->effective_gid = rgid;
@@ -91,7 +90,6 @@ common_setregid(gid_t rgid, gid_t egid, bool setAllIfPrivileged, bool kernel)
} }
// Getting here means all checks were successful -- set the gids. // Getting here means all checks were successful -- set the gids.
InterruptsSpinLocker schedulerLocker(gSchedulerLock);
team->real_gid = rgid; team->real_gid = rgid;
team->effective_gid = egid; team->effective_gid = egid;
team->saved_set_gid = ssgid; team->saved_set_gid = ssgid;
@@ -119,7 +117,6 @@ common_setreuid(uid_t ruid, uid_t euid, bool setAllIfPrivileged, bool kernel)
// setuid() semantics: If privileged set both, real, effective and // setuid() semantics: If privileged set both, real, effective and
// saved set-uid, otherwise set the effective uid. // saved set-uid, otherwise set the effective uid.
if (privileged) { if (privileged) {
InterruptsSpinLocker schedulerLocker(gSchedulerLock);
team->saved_set_uid = ruid; team->saved_set_uid = ruid;
team->real_uid = ruid; team->real_uid = ruid;
team->effective_uid = ruid; team->effective_uid = ruid;
@@ -156,7 +153,6 @@ common_setreuid(uid_t ruid, uid_t euid, bool setAllIfPrivileged, bool kernel)
} }
// Getting here means all checks were successful -- set the uids. // Getting here means all checks were successful -- set the uids.
InterruptsSpinLocker schedulerLocker(gSchedulerLock);
team->real_uid = ruid; team->real_uid = ruid;
team->effective_uid = euid; team->effective_uid = euid;
team->saved_set_uid = ssuid; team->saved_set_uid = ssuid;
@@ -253,8 +249,6 @@ common_setgroups(int groupCount, const gid_t* groupList, bool kernel)
void void
inherit_parent_user_and_group(Team* team, Team* parent) inherit_parent_user_and_group(Team* team, Team* parent)
{ {
InterruptsSpinLocker schedulerLocker(gSchedulerLock);
team->saved_set_uid = parent->saved_set_uid; team->saved_set_uid = parent->saved_set_uid;
team->real_uid = parent->real_uid; team->real_uid = parent->real_uid;
team->effective_uid = parent->effective_uid; team->effective_uid = parent->effective_uid;
@@ -262,8 +256,6 @@ inherit_parent_user_and_group(Team* team, Team* parent)
team->real_gid = parent->real_gid; team->real_gid = parent->real_gid;
team->effective_gid = parent->effective_gid; team->effective_gid = parent->effective_gid;
schedulerLocker.Unlock();
malloc_referenced_acquire(parent->supplementary_groups); malloc_referenced_acquire(parent->supplementary_groups);
team->supplementary_groups = parent->supplementary_groups; team->supplementary_groups = parent->supplementary_groups;
team->supplementary_group_count = parent->supplementary_group_count; team->supplementary_group_count = parent->supplementary_group_count;
@@ -279,7 +271,6 @@ update_set_id_user_and_group(Team* team, const char* file)
return status; return status;
TeamLocker teamLocker(team); TeamLocker teamLocker(team);
InterruptsSpinLocker schedulerLocker(gSchedulerLock);
if ((st.st_mode & S_ISUID) != 0) { if ((st.st_mode & S_ISUID) != 0) {
team->saved_set_uid = st.st_uid; team->saved_set_uid = st.st_uid;
@@ -300,8 +291,6 @@ _kern_getgid(bool effective)
{ {
Team* team = thread_get_current_thread()->team; Team* team = thread_get_current_thread()->team;
InterruptsSpinLocker schedulerLocker(gSchedulerLock);
return effective ? team->effective_gid : team->real_gid; return effective ? team->effective_gid : team->real_gid;
} }
@@ -311,8 +300,6 @@ _kern_getuid(bool effective)
{ {
Team* team = thread_get_current_thread()->team; Team* team = thread_get_current_thread()->team;
InterruptsSpinLocker schedulerLocker(gSchedulerLock);
return effective ? team->effective_uid : team->real_uid; return effective ? team->effective_uid : team->real_uid;
} }
@@ -353,8 +340,6 @@ _user_getgid(bool effective)
{ {
Team* team = thread_get_current_thread()->team; Team* team = thread_get_current_thread()->team;
TeamLocker teamLocker(team);
return effective ? team->effective_gid : team->real_gid; return effective ? team->effective_gid : team->real_gid;
} }
@@ -364,8 +349,6 @@ _user_getuid(bool effective)
{ {
Team* team = thread_get_current_thread()->team; Team* team = thread_get_current_thread()->team;
TeamLocker teamLocker(team);
return effective ? team->effective_uid : team->real_uid; return effective ? team->effective_uid : team->real_uid;
} }
@@ -395,12 +378,9 @@ ssize_t
_user_setgroups(int groupCount, const gid_t* groupList) _user_setgroups(int groupCount, const gid_t* groupList)
{ {
// check privilege // check privilege
{ Team* team = thread_get_current_thread()->team;
Team* team = thread_get_current_thread()->team; if (!is_privileged(team))
TeamLocker teamLocker(team); return EPERM;
if (!is_privileged(team))
return EPERM;
}
return common_setgroups(groupCount, groupList, false); return common_setgroups(groupCount, groupList, false);
} }