Skip to content

on-prem locks: hold on Ubuntu 26.04 (uutils mkdir) and never break a live writer's lock - #1922

Open
abrichr wants to merge 2 commits into
mainfrom
agent/142-audit-lock
Open

abrichr wants to merge 2 commits into
mainfrom
agent/142-audit-lock

Conversation

@abrichr

@abrichr abrichr commented Nov 4, 2026 •

Copy link
Copy Markdown
Member

Found while testing #1918 (on-prem offline install).

What this fixes

The on-prem audit log is a hash chain. audit-log.sh serializes writers with a lock, and the release updater (lib-release.sh) uses a similar lock so that two updates or rollbacks can't run at once. Both locks treated a zero exit status from mkdir LOCKDIR as ownership.

That isn't safe on Ubuntu 26.04, which is the image GitHub's runner used for #1918. Its /usr/bin/mkdir is uutils (the Rust coreutils). In the version it ships, when two processes race to create the same directory, the loser's EEXIST is swallowed and both exit 0. A plain mkdir on a directory that already exists still fails, so this only shows up under contention. Others have reported the same behavior (gaia-harness/gaia#3084 measured double success in 236 of 300 two-way races on ubuntu:26.04 with GNU mkdir at 0 of 300; cahenesy/throughline#148). As a result:

  • Concurrent audit writers could all "hold" the lock, chain to the same parent and break the chain. That is the "concurrent audit writers preserve one hash chain" failure on on-prem: install, update and roll back the real engine offline in CI #1918's CI, which recurred after the first commit here ("audit chain broken at line 20").
  • A second update could proceed while another update or rollback was live.

The change

  • Ownership is now the exclusive creation of the pid file inside the lock directory, done by bash itself with noclobber (set -C, which opens with O_CREAT|O_EXCL). The mkdir -p of the lock directory is no longer what grants the lock. The on-disk layout (LOCK/pid) is unchanged, so an existing holder is still recognized.
  • Stale locks (owner process gone) are broken only under a second exclusive lock that serializes breakers, and only if the lock still names the same dead owner when re-read. This was the first commit here; it closes a separate race where a breaker could delete a lock a live writer had just taken. The breaker lock is now also a noclobber file (audit.log.lock-breaker, .update.lock-breaker). The update lock gets the same compare-before-remove rule; previously it did an unserialized rmdir and mkdir.
  • A symlinked update lock is refused.

Tests (test-update.sh)

  • A mkdir shim that never reports EEXIST models the uutils behavior on any OS.
    • With a live owner holding the update lock, an update under the shim is refused and the owner's lock is untouched.
    • 12 concurrent audit writers under the shim leave an intact 12-record chain.
  • A shimmed cat hands the lock from a dead owner to a live process right after the writer reads it; the live writer's lock must survive (first commit).
  • When a chain check fails, the test prints the verifier output.

Testing

  • macOS: bash deploy/on-prem/bin/test-update.sh, 49 passed, 0 failed (3 runs).
  • Linux (Debian, bash 5.2): 51 passed, 0 failed, 3 runs with GNU coreutils and 3 runs with uutils 0.12.0 first in PATH. Some checks are platform-gated, so the totals differ by OS.
  • Control: the new test file with the previous audit-log.sh and lib-release.sh fails all 3 new checks. The update under the shim goes through, which also fails 2 later rollback checks (44 passed, 5 failed).
  • Stress on Linux pinned to 2 CPUs, 12 concurrent writers per round with the EEXIST shim: the previous code broke the chain in 5 of 5 rounds and this code in 0 of 40.
  • A lock left by a dead owner is still reclaimed at once, for both locks, and both lock directories are removed on release.
  • shellcheck is clean on the changed files apart from an existing SC1091 info.

Not tested

A writer that found the audit lock owned by an exited process removed it
without re-checking. If the owner released between the read and the
removal and another writer took the lock, the breaker deleted the live
lock and two writers could chain to the same parent. Break stale locks
only under a second lock and only if the lock still names the same dead
owner. Add a deterministic test that forces the handover, and print the
verifier output when the concurrent-writer check fails.
@abrichr abrichr added the review-needed Needs an independent review from another vendor's agent label Nov 4, 2026
…r(1)

Ubuntu 26.04 ships uutils mkdir, which reports success to both callers when
two processes race to create one directory. The audit-log writer lock and the
release update lock both treated a zero exit from mkdir as ownership, so on
that image concurrent audit writers chained to the same parent (the
"concurrent audit writers preserve one hash chain" CI failure) and a second
update could proceed beside a live one.

Ownership is now the shell's own noclobber create of the pid file inside the
lock directory (O_CREAT|O_EXCL), and the stale-lock breaker lock is an
exclusive file too. Tests model the uutils behavior with a mkdir shim that
never reports EEXIST; on the previous code they fail.
@abrichr abrichr changed the title fix(on-prem): don't break an audit lock a live writer just took on-prem locks: hold on Ubuntu 26.04 (uutils mkdir) and never break a live writer's lock Nov 5, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review-needed Needs an independent review from another vendor's agent

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant