fix(lock): heap OOB write in __lock_vec + eliminate the lock-mode enumeration class (#140)

[gburd]

Summary

Fixes #140: a heap out-of-bounds write in __lock_vec and the incomplete
replication commit lock list that follows from it. Also eliminates the
root-cause class — adding a DB_LOCK_* mode without auditing the
pre-existing sites that enumerate lock modes — with a CI-enforced guard.

The bug reproduces exactly as reported. Verified before/after under ASan:

BEFORE: WRITE of size 8 at 0x7c3ff701bef8 thread T0
          #0 __lock_vec  src/lock/lock.c:457:15
          #1 __txn_commit src/txn/txn.c:871:11
        0x7c3ff701bef8 is located 0 bytes after 40-byte region
        SUMMARY: AddressSanitizer: heap-buffer-overflow src/lock/lock.c:457 in __lock_vec
AFTER:  clean (both trigger and control)

And the isolation half — the commit lock list content:

	reporter (pre-fix)	this PR
page the txn modified	179	179
page in the commit lock list	140 (the SIREAD object)	179 ✅

Part 1 — the fix, and why this one

__lock_vec's DB_LOCK_PUT_READ path sized the descriptor array from
sh_locker->nwrites, while the population loop skipped only the modes it named
by hand (DB_LOCK_READ, DB_LOCK_READ_UNCOMMITTED). DB_LOCK_SIREAD matched
neither those names nor IS_WRITELOCK, so it fell through and consumed a slot
the sizing never allocated.

The brief listed two candidate fixes. I took (b) — exclude SIREAD from the
list — and rejected (a) sizing by nlocks, because (a) fixes only the
overflow and leaves the isolation bug in place: the list would then be
correctly sized but would still contain SIREAD objects, and
__rep_process_txn reacquires every listed object as DB_LOCK_WRITE. Handing
apply a write lock on a page nobody wrote is at best spurious blocking, at
worst a new correctness problem. A SIREAD marker is an SSI read marker; it has
no business in a list whose purpose is "reacquire the write locks".

Three changes, so that sizing and population agree by construction rather
than by coincidence:

1. Size from the same predicate the loop uses — count IS_WRITELOCK locks
   on heldby. Not nwrites: that counter only counts write locks whose
   status is DB_LSTAT_HELD, and says nothing about the non-write modes the
   loop retains.
2. Gate the populate branch on IS_WRITELOCK(lp->mode) rather than adding
   DB_LOCK_SIREAD to the skip list. Naming one more mode would have fixed
   this bug and left the next one; IS_WRITELOCK covers every future mode.
3. Pass __lock_fix_list the count actually populated (np - data), not a
   separately maintained counter.

What I deliberately did NOT change: the set of modes this path releases.
SIREAD markers must stay on heldby past DB_LOCK_PUT_READ so
__lock_sicommit can persist-or-drop them at __txn_end. Releasing them here
would have been a smaller diff and would have broken SSI. The bug was never
"SIREAD is retained" — it was that a retained non-write lock silently entered a
list sized only for write locks.

The DB_ASSERT bounds check is promoted to a real runtime guard
(__env_panic), since a future sizing/population skew is a memory-safety bug
precisely in the builds where DB_ASSERT is absent.

Part 2 — the audit (19 sites)

Site	Modes handled / mechanism	Verdict
lock.c __lock_vec	objlist sizing, population, fix_list count	FIXED — all three now keyed on IS_WRITELOCK
lock_stat.c __lock_printlock	switch over all modes	FIXED — SIREAD printed as UNKNOWN
db_pr.c __db_lockmode_to_string	switch over all modes	FIXED — SIREAD printed as UNKNOWN LOCK MODE
lock_stat.c __lock_dump_object	walked holders+waiters only	FIXED — an object pinned solely by SIREAD markers printed as empty; now walks sireaders
lock.c __lock_get_internal	nwrites++ under IS_WRITELOCK; SIREAD via safe_si arms	correct
lock.c __lock_freelock	nlocks/nwrites under IS_WRITELOCK	correct
lock.c __lock_downgrade	nwrites-- only on write→non-write	correct
lock.c __lock_inherit_locks	parent counts under IS_WRITELOCK	correct
lock.c __lock_trade	new-locker counts under IS_WRITELOCK	correct
lock.c __lock_put_internal	SIREAD→sireaders, else holders	correct — mode-specific by design
lock.c __lock_sicommit	walks heldby for SIREAD only	correct
lock.c __lock_siclean_obj	walks sireaders (all SIREAD)	correct
lock_deadlock.c __dd_build	switches on detector atype, not on lock mode	correct — not a mode enumeration
lock_failchk.c __lock_failchk	nlocks == nwrites ⇒ "no non-write locks"	correct — SIREAD counts in nlocks only, so a marker-holding locker is not skipped
lock_region.c __lock_region_init	db_riw_conflicts 10×10 incl. SI row+column	correct — SI row/col all-zero; lock_mode >= nmodes rejects an unmatrixed mode
lock_list.c __lock_fix_list	DBT objects, mode-agnostic	correct
lock_list.c __lock_get_list	reacquires in caller's mode	correct
db_meta.c __db_lget	converts READ→SIREAD; coupling tests name single modes	correct — this is where SIREAD is produced
db_meta.c __db_lput	couple/downgrade per named mode	correct — SIREAD intentionally excluded, must be held to txn end
txn_util.c __txn_doevents	IS_WRITELOCK on handle locks	correct — handle locks are never SIREAD

Also checked and found not to be mode enumerations: lock_util.c (hashing
only), lock_id.c (nlocks/nwrites init + the si_ref deferral),
__lock_promote (conflict matrix), lang/tcl/tcl_lock.c (exposes only the 6
classic modes to Tcl by design). Every allocation in src/lock/ was inspected;
__lock_vec was the only one keyed on a mode-dependent count.

Each verdict, with its reason, is committed in
dist/cocci/lockmode_inventory.txt so it can be re-checked rather than
re-derived.

Part 3 — the permanent guard

Coccinelle (dist/cocci/rule_lock_mode_enum.cocci), wired into the existing
baseline gate so NEW matches fail CI:

• LOCK_MODE_SIZING — an allocation sized from ->nwrites. Verified to
  match the libdb 5.3.34: incomplete replication commit lock lists can violate client transaction isolation #140 bug exactly on the pre-fix source, and at zero after it.
• LOCK_MODE_READTEST — a hand-enumerated read-mode test. 5 existing sites,
  all deliberately mode-specific, baselined.

Honest limitation: Coccinelle cannot express "a switch over
db_lockmode_t that is missing a case" in this spatch build — ... when != case X: inside a switch is a parse error (spatch 1.3.1). I tried several
formulations. So the switch check lives in the alternative deliverable:

Checked inventory (dist/cocci/lockmode_inventory.{sh,txt}) — blocking and
deliberately not baselined. Three hard checks:

1. db_lockmode_t in db.in must exactly match the recorded mode set.
2. Every inventoried site must still exist (catches silent renames).
3. Every switch marked exhaustive must have a case arm for every mode.

Each check is verified to fail on a synthetic regression and pass when
restored:

TEST 1 (add DB_LOCK_FROB=10):   FAIL: db_lockmode_t differs from inventory
                                FAIL: __lock_printlock ... has no 'case DB_LOCK_FROB:'
                                FAIL: __db_lockmode_to_string ... no 'case DB_LOCK_FROB:'
TEST 2 (delete SIREAD arm):     FAIL: __lock_printlock ... has no 'case DB_LOCK_SIREAD:'
TEST 3 (rename a site):         FAIL: inventory site function gone: __lock_dump_object
restored:                       OK (10 modes, 19 sites, 2 exhaustive switches)

ASan job (lock-mode-asan) — the empirical half: builds an ASan libdb and
runs test/c/chk.locksireads, asserting no heap overflow and that the
serialized commit lock list names the modified page.

rfc/0003/lock-mode-audit.md — the rule ("a write-lock counter must never
size a buffer that a mode-enumerating loop fills"), the shape to write, a
7-item checklist for adding a mode, and what each guard enforces.

Validation

Check	Result
--enable-debug --enable-test	clean
--enable-debug --enable-diagnostic	clean
release (CFLAGS=-O2)	clean
ASan (-fsanitize=address)	clean
test/c/chk.locksireads	FAILS before the fix, PASSES after (source-only revert, test kept)
ssi001–ssi009	9/9 PASS
txn001/002/003, lock001/002/003	6/6 PASS
test001 btree/hash/queue/recno	4/4 PASS
test/fuzz/check-crashes.sh	9/9 PASS
rep001 btree	PASS
Coccinelle new violations	0
lock-mode inventory	OK
dist/s_include header drift	none

Message ID 2056 is the next free in the lock/mutex range.

Fixes #140

[github-actions]

ABI diff produced no report (build skipped or no base tag).

---

Advisory: libabigail/nm is the authoritative binary-ABI check; Coccinelle is complementary source-level early warning. See dist/cocci/README.md.

[github-actions]

Coccinelle convention checks

No new violations. ✅

Resolved since baseline (2) -- update dist/cocci/baseline.txt to lock these in.

rule_mutex_unbalanced|MUTEX_UNBALANCED|src/crypto/mersenne/mt19937db.c|return (ret);
rule_mutex_unbalanced|MUTEX_UNBALANCED|src/mp/mp_register.c|return (ret);