[PATCH 0/1] Avoid race in perform_groupmems()

[PATCH 0/1] Avoid race in perform_groupmems()

[Peter Kjellerstedt]

We have seen sporadic failures in our CI builds due to useradd,
groupadd or groupmems failing. I have now tracked it down to
perform_groupmems() in useradd_base.bbclass, where I found code to
temporarily create the /etc/gshadow file in the sysroot if it did not
exist. Purportedly this was due to groupmems requiring the gshadow
file to exist. There are, however, no real evidence of that
requirement, and actually creating and removing the file the way it is
currently done WILL cause failures when one task removes it while
another task expects it to already exist.

//Peter

The following changes since commit 53b1fa2d3e1244c46a705d9a40ca0dbee92eab98:

  bitbake: build: Allow dirs/cleandirs to work for empty functions (2016-09-15 12:15:07 +0100)

are available in the git repository at:

  git://git.yoctoproject.org/poky-contrib pkj/groupmems
  http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=pkj/groupmems

Peter Kjellerstedt (1):
  useradd_base.bbclass: Do not mess with the gshadow file in the sysroot

 meta/classes/useradd_base.bbclass | 12 ------------
 1 file changed, 12 deletions(-)

-- 
2.9.0

[PATCH 1/1] useradd_base.bbclass: Do not mess with the gshadow file in the sysroot

[Peter Kjellerstedt]

Previously, if the gshadow file did not exist in the sysroot when
perform_groupmems() was run, it would be temporarily created and
removed again afterwards. This was supposedly due to groupmems failing
if it does not exist.

However, based on empirical testing and examination of the source code
for groupmems, it should not fail if the gshadow file does not exist
when groupmems is started. But it WILL fail if the file is removed
sometime after its existence has been check at the beginning of the
execution, but before it needs to be modified. And this is exactly
what the previous code in perform_groupmems() could cause if multiple
tasks simultaneously modified users or groups. It could cause any of
the useradd, groupadd and groupmems commands to fail as long as at
least one other recipe invoked perform_groupmems().

Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
---
 meta/classes/useradd_base.bbclass | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/meta/classes/useradd_base.bbclass b/meta/classes/useradd_base.bbclass
index f4dc713..ba87edc 100644
--- a/meta/classes/useradd_base.bbclass
+++ b/meta/classes/useradd_base.bbclass
@@ -51,14 +51,6 @@ perform_groupmems () {
 	local groupname=`echo "$opts" | awk '{ for (i = 1; i < NF; i++) if ($i == "-g" || $i == "--group") print $(i+1) }'`
 	local username=`echo "$opts" | awk '{ for (i = 1; i < NF; i++) if ($i == "-a" || $i == "--add") print $(i+1) }'`
 	bbnote "${PN}: Running groupmems command with group $groupname and user $username"
-	# groupmems fails if /etc/gshadow does not exist
-	local gshadow=""
-	if [ -f $rootdir${sysconfdir}/gshadow ]; then
-		gshadow="yes"
-	else
-		gshadow="no"
-		touch $rootdir${sysconfdir}/gshadow
-	fi
 	local mem_exists="`grep "^$groupname:[^:]*:[^:]*:\([^,]*,\)*$username\(,[^,]*\)*" $rootdir/etc/group || true`"
 	if test "x$mem_exists" = "x"; then
 		eval flock -x $rootdir${sysconfdir} -c \"$PSEUDO groupmems \$opts\" || true
@@ -69,10 +61,6 @@ perform_groupmems () {
 	else
 		bbnote "${PN}: group $groupname already contains $username, not re-adding it"
 	fi
-	if test "x$gshadow" = "xno"; then
-		rm -f $rootdir${sysconfdir}/gshadow
-		rm -f $rootdir${sysconfdir}/gshadow-
-	fi
 }
 
 perform_groupdel () {
-- 
2.9.0