On 16/11/11 14:16, Dan McGee wrote:
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 and not over the original database directly. 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, zip the file to the same directory, prefixing the filename with '.tmp.'. We then move the file into place. This move is guaranteed to be atomic, so any reader of the database file will get either the old version, the new version, or ENOENT.
We also perform a hardlink if possible instead of a move when shifting the old database out of the way to '.old'; 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 the 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@archlinux.org>
Minor correction below. Otherwise, Signed-off-by: Allan
--- scripts/repo-add.sh.in | 38 ++++++++++++++++++++++++++------------ 1 files changed, 26 insertions(+), 12 deletions(-)
diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index 280c024..42c7334 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -640,37 +640,51 @@ if (( success )); then msg "$(gettext "Creating updated database file '%s'")" "$REPO_DB_FILE"
TAR_OPT=$(verify_repo_extension "$REPO_DB_FILE") + # $LOCKFILE is already guaranteed to be absolute so this is safe + dirname=${LOCKFILE%/*} filename=${REPO_DB_FILE##*/} + # this ensures we create it on the same filesystem, making moves atomic + tempname="$dirname/.tmp.$filename"
pushd "$tmpdir/tree">/dev/null if ( shopt -s nullglob; files=(*); (( ${#files[*]} )) ); then - bsdtar -c${TAR_OPT}f "$tmpdir/$filename" * + bsdtar -c${TAR_OPT}f "$tempname" * else # we have no packages remaining? zip up some emptyness warning "$(gettext "No packages remain, creating empty database.")" - bsdtar -c${TAR_OPT}f "$tmpdir/$filename" -T /dev/null + bsdtar -c${TAR_OPT}f "$tempname" -T /dev/null fi popd>/dev/null
- create_signature "$tmpdir/$filename" + create_signature "$tempname"
- [[ -f $REPO_DB_FILE ]]&& mv -f "$REPO_DB_FILE" "${REPO_DB_FILE}.old" + # hardlink or move the previous version of the database and signature to .old + # extension as a backup measure + 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" + + # rotate the newly-created database and signature into place + mv "$tempname" "$REPO_DB_FILE" + if [[ -f .sig ]]; then
[[ -f $tempname.sig ]]
+ mv "$tempname.sig" "$REPO_DB_FILE.sig" + fi + dblink="${REPO_DB_FILE%.tar*}" - target=${REPO_DB_FILE##*/} rm -f "$dblink" "$dblink.sig" - ln -s "$target" "$dblink" 2>/dev/null || \ - ln "$target" "$dblink" 2>/dev/null || \ + ln -s "$filename" "$dblink" 2>/dev/null || \ + ln "$filename" "$dblink" 2>/dev/null || \ cp "$REPO_DB_FILE" "$dblink" if [[ -f "$REPO_DB_FILE.sig" ]]; then - ln -s "$target.sig" "$dblink.sig" 2>/dev/null || \ - ln "$target.sig" "$dblink.sig" 2>/dev/null || \ + ln -s "$filename.sig" "$dblink.sig" 2>/dev/null || \ + ln "$filename.sig" "$dblink.sig" 2>/dev/null || \ cp "$REPO_DB_FILE.sig" "$dblink.sig" fi else