Remove race in enewuser, enewgroup
We had an obvious TOCTOU problem in enewuser and enewgroup.
The code would check to see if a user or group was already
created early in these functions, then do a bunch of work
and check again. If the account was created in the meantime,
it'd barf on the second check. This check is redundant anyway
so remove it.
BUG=chromium:378270
TEST=remove accounts for a package and emerge it again
TEST=release trybot
Change-Id: If522df57468cd79466c27945b2240d9bc11ca503
Previous-Reviewed-on: https://chromium-review.googlesource.com/201831
(cherry picked from commit c6778c492f5e75819c316e75f2aae44f33534e4e)
Reviewed-on: https://chromium-review.googlesource.com/202923
Reviewed-by: Chris Masone <cmasone@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
diff --git a/eclass/user.eclass b/eclass/user.eclass
index 12e1b20..8f25c93 100644
--- a/eclass/user.eclass
+++ b/eclass/user.eclass
@@ -190,8 +190,11 @@
# Need to check if the acct exists while we hold the lock, in case
# another ebuild added it in the meantime.
local key=$(awk -F':' '{ print $1 }' <<<"${entry}")
- if [[ -z $(egetent --nolock "${db}" "${key}" "${root}") ]] ; then
+ local existing_entry=$(egetent --nolock "${db}" "${key}" "${root}")
+ if [[ -z ${existing_entry} ]] ; then
echo "${entry}" >> "${dbfile}" || die "Could not write ${entry} to ${dbfile}."
+ else
+ einfo "'${entry}' superceded by '${existing_entry}'"
fi
rm "${lockfile}" || die "Failed to release lock on ${lockfile}."
@@ -242,13 +245,6 @@
enewuser() {
_assert_pkg_ebuild_phase ${FUNCNAME}
- local ACCOUNTS_DIRS
- _find_accounts_dirs
- if [[ ${#ACCOUNTS_DIRS[@]} -eq 0 ]] ; then
- ewarn "No user/group data files present. Skipping."
- return 0
- fi
-
# get the username
local euser=$1; shift
if [[ -z ${euser} ]] ; then
@@ -261,6 +257,14 @@
return 0
fi
+ # Locate all applicable accounts profiles.
+ local ACCOUNTS_DIRS
+ _find_accounts_dirs
+ if [[ ${#ACCOUNTS_DIRS[@]} -eq 0 ]] ; then
+ ewarn "No user/group data files present. Skipping."
+ return 0
+ fi
+
# Ensure username exists in profile.
if [[ -z $(_get_value_for_user "${euser}" user) ]] ; then
die "'${euser}' does not exist in profile!"
@@ -303,10 +307,6 @@
# If profile has no entry w/UID and caller specified one, OK.
fi
- if [[ -n $(egetent passwd ${euid}) ]] ; then
- eerror "UID ${euid} already taken!"
- die "${euid} already taken in $(egetent passwd ${euid})"
- fi
einfo " - Userid: ${euid}"
# See if there's a provided gid and use it if so.
@@ -386,13 +386,6 @@
enewgroup() {
_assert_pkg_ebuild_phase ${FUNCNAME}
- local ACCOUNTS_DIRS
- _find_accounts_dirs
- if [[ ${#ACCOUNTS_DIRS[@]} -eq 0 ]] ; then
- ewarn "No user/group data files present. Skipping."
- return 0
- fi
-
# Get the group.
local egroup=$1; shift
if [[ -z ${egroup} ]] ; then
@@ -404,6 +397,14 @@
if [[ -n $(egetent group "${egroup}") ]] ; then
return 0
fi
+
+ # Locate all applicable accounts profiles.
+ local ACCOUNTS_DIRS
+ _find_accounts_dirs
+ if [[ ${#ACCOUNTS_DIRS[@]} -eq 0 ]] ; then
+ ewarn "No user/group data files present. Skipping."
+ return 0
+ fi
# Ensure group exists in profile.
if [[ -z $(_get_value_for_group "${egroup}" group) ]] ; then
die "Config for ${egroup} not present in profile!"
@@ -446,10 +447,6 @@
die "${egid} conflicts with provided ${provided_gid}!"
fi
fi
- if [[ -n $(egetent group ${egid}) ]] ; then
- eerror "Groupid ${egid} already taken!"
- die "${egid} already taken in $(egetent group ${egid})"
- fi
einfo " - Groupid: ${egid}"
# Handle extra.
