Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 9 Jun 2021 21:20:52 GMT
From:      John Baldwin <jhb@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: 5452cb6bf9f0 - stable/13 - etcupdate: Always extract to a temporary tree.
Message-ID:  <202106092120.159LKqhO051374@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch stable/13 has been updated by jhb:

URL: https://cgit.FreeBSD.org/src/commit/?id=5452cb6bf9f07cdc479c24e880a3628fcb3dda49

commit 5452cb6bf9f07cdc479c24e880a3628fcb3dda49
Author:     John Baldwin <jhb@FreeBSD.org>
AuthorDate: 2021-04-20 20:21:42 +0000
Commit:     John Baldwin <jhb@FreeBSD.org>
CommitDate: 2021-06-09 21:19:38 +0000

    etcupdate: Always extract to a temporary tree.
    
    etcupdate has had a somewhat nasty race condition since its creation
    in that its state machine can get very confused if it is interrupted
    while building the tree to compare against.  This is exacerbated by
    the fact that etcupdate doesn't emit any output while building the
    tree which can take several seconds (especially in recent years with
    the addition of the tree-wide buildconfig/installconfig passes).
    
    To mitigate this, always install a new tree into a temporary directory
    created via mktemp as was previously done only for dry-runs via -n.
    The existing trees are only rotated and the new tree installed as
    /var/db/etcupdate/current after the update command has completed.
    
    Reported by:    dim, np (and many others)
    Reviewed by:    imp
    Differential Revision:  https://reviews.freebsd.org/D29843
    
    (cherry picked from commit 0611aec3cf3a373e6a06f103699dbc91c3d6d472)
    (cherry picked from commit b0df36580d5b0df67a0f58ded8f6356b268f7f71)
---
 usr.sbin/etcupdate/etcupdate.sh | 148 +++++++++++++++++++++++++---------------
 1 file changed, 92 insertions(+), 56 deletions(-)

diff --git a/usr.sbin/etcupdate/etcupdate.sh b/usr.sbin/etcupdate/etcupdate.sh
index 87d269f2ca69..41f13412f340 100755
--- a/usr.sbin/etcupdate/etcupdate.sh
+++ b/usr.sbin/etcupdate/etcupdate.sh
@@ -226,9 +226,11 @@ build_tree()
 	return 0
 }
 
-# Generate a new NEWTREE tree.  If tarball is set, then the tree is
+# Generate a new tree.  If tarball is set, then the tree is
 # extracted from the tarball.  Otherwise the tree is built from a
 # source tree.
+#
+# $1 - directory to store new tree in
 extract_tree()
 {
 	local files
@@ -239,16 +241,16 @@ extract_tree()
 		if [ -n "$preworld" ]; then
 			files="$PREWORLD_FILES"
 		fi
-		if ! (mkdir -p $NEWTREE && tar xf $tarball -C $NEWTREE $files) \
+		if ! (mkdir -p $1 && tar xf $tarball -C $1 $files) \
 		    >&3 2>&1; then
 			echo "Failed to extract new tree."
-			remove_tree $NEWTREE
+			remove_tree $1
 			exit 1
 		fi
 	else
-		if ! build_tree $NEWTREE; then
+		if ! build_tree $1; then
 			echo "Failed to build new tree."
-			remove_tree $NEWTREE
+			remove_tree $1
 			exit 1
 		fi
 	fi
@@ -1353,14 +1355,28 @@ extract_cmd()
 
 	log "extract command: tarball=$tarball"
 
+	# Create a temporary directory to hold the tree
+	dir=`mktemp -d $WORKDIR/etcupdate-XXXXXXX`
+	if [ $? -ne 0 ]; then
+		echo "Unable to create temporary directory."
+		exit 1
+	fi
+
+	extract_tree $dir
+
 	if [ -d $NEWTREE ]; then
 		if ! remove_tree $NEWTREE; then
 			echo "Unable to remove current tree."
+			remove_tree $dir
 			exit 1
 		fi
 	fi
 
-	extract_tree
+	if ! mv $dir $NEWTREE >&3 2>&1; then
+		echo "Unable to rename temp tree to current tree."
+		remove_tree $dir
+		exit 1
+	fi
 }
 
 # Resolve conflicts left from an earlier merge.
@@ -1420,7 +1436,7 @@ status_cmd()
 # source tree.
 update_cmd()
 {
-	local dir
+	local dir new old
 
 	if [ $# -ne 0 ]; then
 		usage
@@ -1449,60 +1465,44 @@ update_cmd()
 		exit 1
 	fi
 
+	# Save tree names to use for rotation later.
+	old=$OLDTREE
+	new=$NEWTREE
 	if [ -z "$rerun" ]; then
-		# For a dryrun that is not a rerun, do not rotate the existing
-		# stock tree.  Instead, extract a tree to a temporary directory
-		# and use that for the comparison.
-		if [ -n "$dryrun" ]; then
-			dir=`mktemp -d $WORKDIR/etcupdate-XXXXXXX`
-			if [ $? -ne 0 ]; then
-				echo "Unable to create temporary directory."
-				exit 1
-			fi
-
-			# A pre-world dryrun has already set OLDTREE to
-			# point to the current stock tree.
-			if [ -z "$preworld" ]; then
-				OLDTREE=$NEWTREE
-			fi
-			NEWTREE=$dir
-
-		# For a pre-world update, blow away any pre-existing
-		# NEWTREE.
-		elif [ -n "$preworld" ]; then
-			if ! remove_tree $NEWTREE; then
-				echo "Unable to remove pre-world tree."
-				exit 1
-			fi
+		# Extract the new tree to a temporary directory.  The
+	        # trees are only rotated after a successful update to
+	        # avoid races if an update command is interrupted
+	        # before it completes.
+		dir=`mktemp -d $WORKDIR/etcupdate-XXXXXXX`
+		if [ $? -ne 0 ]; then
+			echo "Unable to create temporary directory."
+			exit 1
+		fi
 
-		# Rotate the existing stock tree to the old tree.
-		elif [ -d $NEWTREE ]; then
-			# First, delete the previous old tree if it exists.
-			if ! remove_tree $OLDTREE; then
-				echo "Unable to remove old tree."
-				exit 1
-			fi
+		# Populate the new tree.
+		extract_tree $dir
 
-			# Move the current stock tree.
-			if ! mv $NEWTREE $OLDTREE >&3 2>&1; then
-				echo "Unable to rename current stock tree."
-				exit 1
-			fi
+		# Compare the new tree against the previous tree.  For
+		# the preworld case OLDTREE already points to the
+		# current stock tree.
+		if [ -z "$preworld" ]; then
+			OLDTREE=$NEWTREE
 		fi
+		NEWTREE=$dir
+	fi
 
-		if ! [ -d $OLDTREE ]; then
-			cat <<EOF
+	if ! [ -d $OLDTREE ]; then
+		cat <<EOF
 No previous tree to compare against, a sane comparison is not possible.
 EOF
-			log "No previous tree to compare against."
-			if [ -n "$dir" ]; then
-				rmdir $dir
+		log "No previous tree to compare against."
+		if [ -n "$dir" ]; then
+			if [ -n "$rerun" ]; then
+				panic "Should not have a temporary directory"
 			fi
-			exit 1
+			remove_tree $dir
 		fi
-
-		# Populate the new tree.
-		extract_tree
+		exit 1
 	fi
 
 	# Build lists of nodes in the old and new trees.
@@ -1568,12 +1568,48 @@ EOF
 		cat $WARNINGS
 	fi
 
-	if [ -n "$dir" ]; then
-		if [ -z "$dryrun" -o -n "$rerun" ]; then
+	# Finally, rotate any needed trees.
+	if [ "$new" != "$NEWTREE" ]; then
+		if [ -n "$rerun" ]; then
 			panic "Should not have a temporary directory"
 		fi
-		
-		remove_tree $dir
+		if [ -z "$dir" ]; then
+			panic "Should have a temporary directory"
+		fi
+
+		# Rotate the old tree if needed
+		if [ "$old" != "$OLDTREE" ]; then
+			if [ -n "$preworld" ]; then
+				panic "Old tree should be unchanged"
+			fi
+
+			if ! remove_tree $old; then
+				echo "Unable to remove previous old tree."
+				exit 1
+			fi
+
+			if ! mv $OLDTREE $old >&3 2>&1; then
+				echo "Unable to rename old tree."
+				exit 1
+			fi
+		fi
+
+		# Rotate the new tree.  Remove a previous pre-world
+		# tree if it exists.
+		if [ -d $new ]; then
+			if [ -z "$preworld" ]; then
+				panic "New tree should be rotated to old"
+			fi
+			if ! remove_tree $new; then
+				echo "Unable to remove previous pre-world tree."
+				exit 1
+			fi
+		fi
+
+		if ! mv $NEWTREE $new >&3 2>&1; then
+			echo "Unable to rename current tree."
+			exit 1
+		fi
 	fi
 }
 



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202106092120.159LKqhO051374>