All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Two useradd fixes
@ 2012-03-23  4:43 Scott Garman
  2012-03-23  4:43 ` [PATCH 1/2] package_rpm.bbclass: ensure base-passwd and shadow get installed first Scott Garman
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Scott Garman @ 2012-03-23  4:43 UTC (permalink / raw)
  To: openembedded-core

Hello,

This pull request fixes two race conditions related to the useradd code.

Yocto bug #2127 describes how when using the RPM package manager, the
order of package installation during generatin of the rootfs did not
guarantee that base-passwd and shadow would be installed before other
packages which need to install custom users/groups in their postinstall
functions. This fix ensures that when base-passwd and/or shadow are
part of an image, they will be installed first during image generation.

The second race condition is described in Yocto bug #1794, and can occur
if two build steps try to update the sysroot passwd/group files at the
same time. Since the shadow utilities enforce file locking, the commands
will fail to run. The fix for this involves checking for failure and
retrying the commands up to 10 times, with a 1s delay between attempts
before giving up.

Verifying these fixes involved creating numerous images and useradd-based
packages repeatedly on a build host with high parallelism, and also doing
so from a pre-populated sstate-cache. I was able to reliably simulate the
file locking behavior by making the passwd/group files read only when
developing the fix for #1794. 

Scott

The following changes since commit 87e581bb7da9f1530d190cd023fcf892c8b858f5:

  ddimage: Add script for writing images to boot media (2012-03-22 19:17:54 +0000)

are available in the git repository at:
  git://git.pokylinux.org/poky-contrib sgarman/useradd-lock-fix-oe
  http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=sgarman/useradd-lock-fix-oe

Scott Garman (2):
  package_rpm.bbclass: ensure base-passwd and shadow get installed
    first
  useradd.bbclass: retry useradd/groupadd commands to avoid lock race
    issues

 meta/classes/package_rpm.bbclass |    9 ++++++++-
 meta/classes/useradd.bbclass     |   36 ++++++++++++++++++++++++++++++++++--
 2 files changed, 42 insertions(+), 3 deletions(-)

-- 
1.7.5.4




^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] package_rpm.bbclass: ensure base-passwd and shadow get installed first
  2012-03-23  4:43 [PATCH 0/2] Two useradd fixes Scott Garman
@ 2012-03-23  4:43 ` Scott Garman
  2012-03-23  4:43 ` [PATCH 2/2] useradd.bbclass: retry useradd/groupadd commands to avoid lock race issues Scott Garman
  2012-03-23 12:04 ` [PATCH 0/2] Two useradd fixes Richard Purdie
  2 siblings, 0 replies; 5+ messages in thread
From: Scott Garman @ 2012-03-23  4:43 UTC (permalink / raw)
  To: openembedded-core

When generating images, we need to make sure that base-passwd and
shadow get installed before other packages, which might need to create
custom user accounts.

Thanks to Richard Purdie for the initial version of this fix.

This fixes [YOCTO #2127]

Signed-off-by: Scott Garman <scott.a.garman@intel.com>
---
 meta/classes/package_rpm.bbclass |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/meta/classes/package_rpm.bbclass b/meta/classes/package_rpm.bbclass
index 80b1619..e83fc55 100644
--- a/meta/classes/package_rpm.bbclass
+++ b/meta/classes/package_rpm.bbclass
@@ -439,7 +439,14 @@ package_install_internal_rpm () {
 
 	fi
 
-	cat ${target_rootfs}/install/install_solution.manifest > ${target_rootfs}/install/total_solution.manifest
+	# If base-passwd or shadow are in the list of packages to install,
+	# ensure they are installed first to support later packages that
+	# may create custom users/groups (fixes Yocto bug #2127)
+	infile=${target_rootfs}/install/install_solution.manifest
+	outfile=${target_rootfs}/install/total_solution.manifest
+	cat $infile | grep /base-passwd-[0-9] > $outfile || true
+	cat $infile | grep /shadow-[0-9] >> $outfile || true
+	cat $infile | grep -v /shadow-[0-9] | grep -v /base-passwd-[0-9] >> $outfile
 	cat ${target_rootfs}/install/install_multilib_solution.manifest >> ${target_rootfs}/install/total_solution.manifest
 
 	# Construct install scriptlet wrapper
-- 
1.7.5.4




^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] useradd.bbclass: retry useradd/groupadd commands to avoid lock race issues
  2012-03-23  4:43 [PATCH 0/2] Two useradd fixes Scott Garman
  2012-03-23  4:43 ` [PATCH 1/2] package_rpm.bbclass: ensure base-passwd and shadow get installed first Scott Garman
@ 2012-03-23  4:43 ` Scott Garman
  2012-03-23 21:12   ` McClintock Matthew-B29882
  2012-03-23 12:04 ` [PATCH 0/2] Two useradd fixes Richard Purdie
  2 siblings, 1 reply; 5+ messages in thread
From: Scott Garman @ 2012-03-23  4:43 UTC (permalink / raw)
  To: openembedded-core

A race condition can occur when adding users and groups to the
passwd and group files, causing errors like the following:

 ERROR: Function 'useradd_sysroot' failed
 Tried to access "/etc/group" but this was locked.

This fix will cause the useradd code to retry the useradd and
groupadd commands up to 10 times (with a 1s sleep in between
attempts) before failing.

This fixes [YOCTO #1794]

Signed-off-by: Scott Garman <scott.a.garman@intel.com>
---
 meta/classes/useradd.bbclass |   36 ++++++++++++++++++++++++++++++++++--
 1 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/meta/classes/useradd.bbclass b/meta/classes/useradd.bbclass
index 7981a68..0bbb371 100644
--- a/meta/classes/useradd.bbclass
+++ b/meta/classes/useradd.bbclass
@@ -45,7 +45,23 @@ if test "x$GROUPADD_PARAM" != "x"; then
 		groupname=`echo "$opts" | awk '{ print $NF }'`
 		group_exists=`grep "^$groupname:" $SYSROOT/etc/group || true`
 		if test "x$group_exists" = "x"; then
-			eval $PSEUDO groupadd  $OPT $opts
+			count=1
+			while true; do
+				eval $PSEUDO groupadd $OPT $opts || true
+				group_exists=`grep "^$groupname:" $SYSROOT/etc/group || true`
+				if test "x$group_exists" = "x"; then
+					# File locking issues can require us to retry the command
+					echo "WARNING: groupadd command did not succeed. Retrying..."
+					sleep 1
+				else
+					break
+				fi
+				count=`expr $count + 1`
+				if test $count = 11; then
+					echo "ERROR: tried running groupadd command 10 times without success, giving up"
+					exit 1
+				fi
+			done		
 		else
 			echo "Note: group $groupname already exists, not re-creating it"
 		fi
@@ -70,7 +86,23 @@ if test "x$USERADD_PARAM" != "x"; then
 		username=`echo "$opts" | awk '{ print $NF }'`
 		user_exists=`grep "^$username:" $SYSROOT/etc/passwd || true`
 		if test "x$user_exists" = "x"; then
-			eval $PSEUDO useradd $OPT $opts
+			count=1
+			while true; do
+				eval $PSEUDO useradd $OPT $opts || true
+				user_exists=`grep "^$username:" $SYSROOT/etc/passwd || true`
+				if test "x$user_exists" = "x"; then
+					# File locking issues can require us to retry the command
+					echo "WARNING: useradd command did not succeed. Retrying..."
+					sleep 1
+				else
+					break
+				fi
+				count=`expr $count + 1`
+				if test $count = 11; then
+					echo "ERROR: tried running useradd command 10 times without success, giving up"
+					exit 1
+				fi
+			done
 		else
 			echo "Note: username $username already exists, not re-creating it"
 		fi
-- 
1.7.5.4




^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/2] Two useradd fixes
  2012-03-23  4:43 [PATCH 0/2] Two useradd fixes Scott Garman
  2012-03-23  4:43 ` [PATCH 1/2] package_rpm.bbclass: ensure base-passwd and shadow get installed first Scott Garman
  2012-03-23  4:43 ` [PATCH 2/2] useradd.bbclass: retry useradd/groupadd commands to avoid lock race issues Scott Garman
@ 2012-03-23 12:04 ` Richard Purdie
  2 siblings, 0 replies; 5+ messages in thread
From: Richard Purdie @ 2012-03-23 12:04 UTC (permalink / raw)
  To: Patches and discussions about the oe-core layer

On Thu, 2012-03-22 at 21:43 -0700, Scott Garman wrote:
> Hello,
> 
> This pull request fixes two race conditions related to the useradd code.
> 
> Yocto bug #2127 describes how when using the RPM package manager, the
> order of package installation during generatin of the rootfs did not
> guarantee that base-passwd and shadow would be installed before other
> packages which need to install custom users/groups in their postinstall
> functions. This fix ensures that when base-passwd and/or shadow are
> part of an image, they will be installed first during image generation.
> 
> The second race condition is described in Yocto bug #1794, and can occur
> if two build steps try to update the sysroot passwd/group files at the
> same time. Since the shadow utilities enforce file locking, the commands
> will fail to run. The fix for this involves checking for failure and
> retrying the commands up to 10 times, with a 1s delay between attempts
> before giving up.
> 
> Verifying these fixes involved creating numerous images and useradd-based
> packages repeatedly on a build host with high parallelism, and also doing
> so from a pre-populated sstate-cache. I was able to reliably simulate the
> file locking behavior by making the passwd/group files read only when
> developing the fix for #1794. 
> 
> Scott
> 
> The following changes since commit 87e581bb7da9f1530d190cd023fcf892c8b858f5:
> 
>   ddimage: Add script for writing images to boot media (2012-03-22 19:17:54 +0000)
> 
> are available in the git repository at:
>   git://git.pokylinux.org/poky-contrib sgarman/useradd-lock-fix-oe
>   http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=sgarman/useradd-lock-fix-oe
> 
> Scott Garman (2):
>   package_rpm.bbclass: ensure base-passwd and shadow get installed
>     first
>   useradd.bbclass: retry useradd/groupadd commands to avoid lock race
>     issues

Merged to master, thanks!

Richard




^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] useradd.bbclass: retry useradd/groupadd commands to avoid lock race issues
  2012-03-23  4:43 ` [PATCH 2/2] useradd.bbclass: retry useradd/groupadd commands to avoid lock race issues Scott Garman
@ 2012-03-23 21:12   ` McClintock Matthew-B29882
  0 siblings, 0 replies; 5+ messages in thread
From: McClintock Matthew-B29882 @ 2012-03-23 21:12 UTC (permalink / raw)
  To: Patches and discussions about the oe-core layer

This should go in our edison 1.1.2 release as well.

-M

On Thu, Mar 22, 2012 at 11:43 PM, Scott Garman <scott.a.garman@intel.com> wrote:
> A race condition can occur when adding users and groups to the
> passwd and group files, causing errors like the following:
>
>  ERROR: Function 'useradd_sysroot' failed
>  Tried to access "/etc/group" but this was locked.
>
> This fix will cause the useradd code to retry the useradd and
> groupadd commands up to 10 times (with a 1s sleep in between
> attempts) before failing.
>
> This fixes [YOCTO #1794]
>
> Signed-off-by: Scott Garman <scott.a.garman@intel.com>
> ---
>  meta/classes/useradd.bbclass |   36 ++++++++++++++++++++++++++++++++++--
>  1 files changed, 34 insertions(+), 2 deletions(-)
>
> diff --git a/meta/classes/useradd.bbclass b/meta/classes/useradd.bbclass
> index 7981a68..0bbb371 100644
> --- a/meta/classes/useradd.bbclass
> +++ b/meta/classes/useradd.bbclass
> @@ -45,7 +45,23 @@ if test "x$GROUPADD_PARAM" != "x"; then
>                groupname=`echo "$opts" | awk '{ print $NF }'`
>                group_exists=`grep "^$groupname:" $SYSROOT/etc/group || true`
>                if test "x$group_exists" = "x"; then
> -                       eval $PSEUDO groupadd  $OPT $opts
> +                       count=1
> +                       while true; do
> +                               eval $PSEUDO groupadd $OPT $opts || true
> +                               group_exists=`grep "^$groupname:" $SYSROOT/etc/group || true`
> +                               if test "x$group_exists" = "x"; then
> +                                       # File locking issues can require us to retry the command
> +                                       echo "WARNING: groupadd command did not succeed. Retrying..."
> +                                       sleep 1
> +                               else
> +                                       break
> +                               fi
> +                               count=`expr $count + 1`
> +                               if test $count = 11; then
> +                                       echo "ERROR: tried running groupadd command 10 times without success, giving up"
> +                                       exit 1
> +                               fi
> +                       done
>                else
>                        echo "Note: group $groupname already exists, not re-creating it"
>                fi
> @@ -70,7 +86,23 @@ if test "x$USERADD_PARAM" != "x"; then
>                username=`echo "$opts" | awk '{ print $NF }'`
>                user_exists=`grep "^$username:" $SYSROOT/etc/passwd || true`
>                if test "x$user_exists" = "x"; then
> -                       eval $PSEUDO useradd $OPT $opts
> +                       count=1
> +                       while true; do
> +                               eval $PSEUDO useradd $OPT $opts || true
> +                               user_exists=`grep "^$username:" $SYSROOT/etc/passwd || true`
> +                               if test "x$user_exists" = "x"; then
> +                                       # File locking issues can require us to retry the command
> +                                       echo "WARNING: useradd command did not succeed. Retrying..."
> +                                       sleep 1
> +                               else
> +                                       break
> +                               fi
> +                               count=`expr $count + 1`
> +                               if test $count = 11; then
> +                                       echo "ERROR: tried running useradd command 10 times without success, giving up"
> +                                       exit 1
> +                               fi
> +                       done
>                else
>                        echo "Note: username $username already exists, not re-creating it"
>                fi
> --
> 1.7.5.4
>
>
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2012-03-23 21:36 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-03-23  4:43 [PATCH 0/2] Two useradd fixes Scott Garman
2012-03-23  4:43 ` [PATCH 1/2] package_rpm.bbclass: ensure base-passwd and shadow get installed first Scott Garman
2012-03-23  4:43 ` [PATCH 2/2] useradd.bbclass: retry useradd/groupadd commands to avoid lock race issues Scott Garman
2012-03-23 21:12   ` McClintock Matthew-B29882
2012-03-23 12:04 ` [PATCH 0/2] Two useradd fixes Richard Purdie

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.