[pacman-dev] [PATCH] repo-add: ensure database and signature files are always viewed in whole

[Dan McGee]

This addresses a short but sweet race condition currently existing in
repo-add and repo-remove. We do the smart thing and zip the database to
a location in a temporary directory. However, we then proceed to move
this file directly from the temporary directory to our final location,
which is more than likely a cross-filesystem move (/tmp on tmpfs) and
thus non-atomic.

Instead, break the move into two parts- first, move the file to a name
with a '.tmp.' prefix in the correct directory, then move it to the
correct name. This second move is guaranteed to be atomic, so any reader
of the database file will get either the old version, the new version,
or a file that doesn't exist.

We also perform a hardlink if possible instead of a move when shifting
the old database out of the way; this ensures there is no chance of a
database file not existing during the whole process.

Only one small race condition should now be present- when the database
has been fully moved into place and th signature has not, you may see a
mismatch. There seems to be no good way to address this, and it existed
before this patch.

A final note- if someone had locked-down permissions on the directory
that the database files are in (e.g., could only write to foo.db.tar.gz,
foo.db, foo.db.tar.gz.old, foo.db.old, and the lock file), this would
break.

Signed-off-by: Dan McGee <dan at archlinux.org>
---

As I was writing the commit message for this I also realized we could just
`bsdtar cf` straight to the '.tmp.' prefix and save ourselves a copy- this
probably makes more sense.

Thoughts on the general ideas of the patch, besides the above simplification?

 scripts/repo-add.sh.in |   20 ++++++++++++++++----
 1 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in
index fc008fc..d3e72fd 100644
--- a/scripts/repo-add.sh.in
+++ b/scripts/repo-add.sh.in
@@ -655,14 +655,26 @@ if (( success )); then
 
 	create_signature "$tmpdir/$filename"
 
-	[[ -f $REPO_DB_FILE ]] && mv -f "$REPO_DB_FILE" "${REPO_DB_FILE}.old"
+	if [[ -f $REPO_DB_FILE ]]; then
+		ln -f "$REPO_DB_FILE" "${REPO_DB_FILE}.old" 2>/dev/null || \
+			mv -f "$REPO_DB_FILE" "${REPO_DB_FILE}.old"
+	fi
 	if [[ -f $REPO_DB_FILE.sig ]]; then
-		mv -f "$REPO_DB_FILE.sig" "$REPO_DB_FILE.old.sig"
+		ln -f "$REPO_DB_FILE.sig" "$REPO_DB_FILE.old.sig" 2>/dev/null || \
+			mv -f "$REPO_DB_FILE.sig" "$REPO_DB_FILE.old.sig"
 	else
 		rm -f "$REPO_DB_FILE.old.sig"
 	fi
-	[[ -f $tmpdir/$filename ]] && mv "$tmpdir/$filename" "$REPO_DB_FILE"
-	[[ -f $tmpdir/$filename.sig ]] && mv "$tmpdir/$filename.sig" "$REPO_DB_FILE.sig"
+
+	# These odd double-moves ensure the database and signature files are seen all
+	# or nothing and not halfway through a filesystem to filesystem copy.
+	mv "$tmpdir/$filename" ".tmp.$REPO_DB_FILE"
+	mv ".tmp.$REPO_DB_FILE" "$REPO_DB_FILE"
+	if [[ -f $tmpdir/$filename.sig ]]; then
+		mv "$tmpdir/$filename.sig" ".tmp.$REPO_DB_FILE.sig"
+		mv ".tmp.$REPO_DB_FILE.sig" "$REPO_DB_FILE.sig"
+	fi
+
 	dblink="${REPO_DB_FILE%.tar*}"
 	target=${REPO_DB_FILE##*/}
 	rm -f "$dblink" "$dblink.sig"
-- 
1.7.7.3