mod_dav_fs: serialize lock database access with a global mutex (backport r1914438)

[manfred-kaiser]

What this changes

mod_dav_fs reads a per-resource lock record from the lock database,
modifies it in memory, and writes the whole record back. There is no
mutual exclusion around that fetch-modify-store cycle: no apr_global_mutex,
no advisory file lock, no compare-and-swap. When APR-util is built against a
DBM backend that does no internal locking of its own, concurrent LOCK
requests on the same resource race on the shared record.

trunk fixed this in r1914438 (git 455147a3, GitHub PR #395: an
apr_global_mutex around the lockdb open..modify..close cycle, released
through a request-pool cleanup), with the 2.4.x-compatibility follow-up
r1914457. The fix was never backported to 2.4.x and is not listed in the
branch STATUS, so every current 2.4.x release is still affected. This PR
backports that locking.

RHEL, CentOS Stream, AlmaLinux and Rocky already ship an equivalent vendor
backport of this fix; it has simply never landed in upstream 2.4.x. This PR
brings the same change upstream.

Which backends are affected

Whether the race is reachable depends on the DBM backend APR-util was built
with. Some serialize access as a side effect; some do not.

Backend	Internal locking	Affected	Mainstream default
sdbm	whole-file fcntl lock around every open/close (APR-util's own wrapper)	no	openSUSE
gdbm	exclusive flock in gdbm_open()	no	none known
Berkeley DB	none	yes	Debian, Ubuntu, RHEL 9 family
ndbm	none	yes	none known

Impact (Berkeley DB)

• Granted locks are dropped from the stored list: a client holds a
  Lock-Token the server issued, but the lock is absent from a later
  PROPFIND lockdiscovery.
• Two clients are both granted an exclusive lock on the same resource,
  which defeats the guarantee exclusive locking exists to provide.
• Under sustained concurrent LOCK load the lock file corrupts
  (DB_PAGE_NOTFOUND, and PANIC: DB_RUNRECOVERY), after which every LOCK
  returns 500 for all users until the server is restarted.

This first showed up as an intermittent failure under ordinary concurrent
test traffic, not a crafted attack. A client with write access can also
trigger it deliberately to break another client's exclusive lock.

Security relevance

This is security-relevant, not only a robustness bug. An authenticated
client with write access can:

• defeat exclusive-lock mutual exclusion (integrity): obtain a second
  exclusive lock on a resource another client has exclusively locked, and
  write through it;
• drive the lock database into corruption (DB_RUNRECOVERY) under
  concurrent LOCK load, taking the whole lock service down with 500 for
  all users until a restart (availability).

The fix has been public in trunk since 2023 (r1914438), so this is not an
undisclosed issue; all current 2.4.x releases are nonetheless still
affected. Because both the issue and its fix are already public, this is
submitted directly as a public backport PR rather than through the private
security process. The security team may still wish to assign a CVE for the
2.4.x exposure; this PR is the backport regardless of that decision.

The change

• mod_dav_fs.c: register and create the global mutex (pre_config,
  post_config, child_init) and store it on the per-server config.
• lock.c: take the mutex in dav_fs_really_open_lockdb(); release it and
  close the DB through an apr_pool_cleanup run from
  dav_fs_close_lockdb().
• repos.h: carry the mutex on dav_fs_server_conf.

Two APLOGNO() tags (10488, 10489) are carried over from r1914438 with
their existing numbers, as is normal for a backport. The change
intentionally omits the DAVLockDBType directive that r1914438 added
alongside the fix: that is a separate feature (it threads a dbm type through
dav_dbm_open_direct() and its callers) and is not needed to close the
race. The full r1914438 is of course also an option if preferred. Only
http_core.h had to be included for ap_state_query; this backport adds no
default lockdb path, so the ap_state_dir_relative guard from the follow-up
r1914457 is not needed here.

Verification

Built against a Berkeley-DB APR-util, compared with stock 2.4.69 on the same
backend. prefork, StartServers 5 (forces mid-run worker spawning, the
realistic default) unless noted. Harness: concurrent real-HTTP LOCK
clients, then a PROPFIND checking that every issued token is present.

Lost locks / exclusive race (integrity)

Shared locks, one fresh resource per round, 40 rounds:

concurrency	stock 2.4.69	with this patch
16 (run 1/2/3)	29 / 29 / 33 rounds lose locks (54 / 59 / 60 locks)	0 / 0 / 0
32	lost in most rounds	0

Exclusive-lock mutual exclusion (PoC: several concurrent exclusive LOCKs on
a never-locked resource): stock grants 2+ winners on the same resource; with
the patch, exactly one.

Corruption / outage (availability)

One reused resource, concurrency 24, 60 rounds:

	stock 2.4.69	with this patch
locks lost	283	0
HTTP 500	866	0
error log	repeated DB_PAGE_NOTFOUND	clean

High concurrent LOCK load, distinct resources (8 s/run)

concurrency	stock ok/s	stock failed	patched ok/s	patched failed
16	656	996	662	0
32	541	1841	608	0
64	617	1044	670	0

Stock produced ~3900 failed requests and 41 DB_PAGE_NOTFOUND log entries
over these runs. Throughput is the same with and without the patch: the
per-op Berkeley DB open/close dominates, not the mutex wait. The mutex does
not cost throughput, it only stops the concurrent corruption. Patched tail
latency (p99) was 85 / 170 / 661 ms at concurrency 16 / 32 / 64, every
request succeeding.

Collection-lock serialization (the one behavioral cost)

The mutex is held across a Depth: infinity collection LOCK for the whole
tree walk, so one large collection operation delays other lock operations
while it runs. Six concurrent 8000-member exclusive collection LOCKs, with
ten unrelated single LOCKs issued during them:

backend / build	collection locks (ms, by queue position)	unrelated single LOCK wait
Berkeley DB, this patch	99, 199, 295, 394, 491, 597	581-590 ms
sdbm, stock 2.4.69 (no patch)	115, 264, 435, 613, 787, 959	951-955 ms, tails to 2103 ms
sdbm, trunk	115, 261, 430, 576, 767, 955	946-952 ms, tails to 2691 ms

This is not a regression introduced by the patch. The sdbm backend, which is
not affected by the race, already serializes this way (APR holds a
whole-file fcntl lock for the open handle's lifetime), and with longer,
less even waits than the patched Berkeley DB path. The patch brings Berkeley
DB to the same, in fact tighter, behavior. Ordinary GET/PUT and
single-resource locking are unaffected.

---

The security-vulnerability checkbox below is left unticked on purpose: the
change is security-relevant (see "Security relevance" above). It is already
public, so this is submitted as a normal public backport rather than through
the private security process.

This is a backport, so two further checklist items do not apply verbatim: the two
APLOGNO() tags (10488, 10489) are carried over from r1914438 with their
existing numbers rather than being empty, and this 2.4.x branch records
user-visible changes in the CHANGES file (there is no changes-entries/
directory here), where an entry is included.

• This PR does not report or submit a patch for a security
  vulnerability (see SECURITY.md
  and https://www.apache.org/security/#reporting-a-vulnerability).
• Code follows the httpd style guide.
• New log messages (level debug or higher) use an empty APLOGNO() tag;
  numbers are assigned by a committer at merge time
  (see docs/log-message-tags/README).
• If the change is user-visible, a changes-entries/*.txt file is included,
  following the template in README.CHANGES (not needed otherwise).

[notroj]

Thanks for the PR, I have this tracked already at #399 and plan to propose it before the next release.