NULL dereference: DB_ENV->rep_get_nsites() and DB_TXN method gaps in __cdsgroup_begin

[gburd]

Two more public-API NULL-dereference paths found while building the coverage drivers in #147.

1. rep_get_nsites() on an unopened repmgr handle

Calling DB_ENV->rep_get_nsites() before the environment is opened dereferences a NULL. The guard that should catch this, ENV_NOT_CONFIGURED, is a no-op before open (src/dbinc/db_int.in:604), so the pre-open case falls straight through. Reproducible in ~12 lines of public API.

2. __cdsgroup_begin installs only 8 of DB_TXN's 12 methods

The CDS group transaction handle leaves get_priority, set_priority, set_commit_token and set_txn_lsnp as NULL function pointers. Calling any of them on a CDS group txn jumps to 0x0 rather than returning an error.

Both are reachable from documented public APIs with no invalid arguments — just an unexpected call order or an access method whose handle is less fully populated than the type implies.

Suggested fix

1. Make the pre-open case an explicit error (or make ENV_NOT_CONFIGURED meaningful before open) for rep_get_nsites and audit its siblings for the same shape.
2. Populate the missing DB_TXN slots in __cdsgroup_begin with functions that return EINVAL/DB_OPNOTSUP, so an unsupported operation is reported rather than crashing. Worth checking every handle constructor that hand-installs a subset of a method table.

Not fixed in #147, which was scoped to tests only.

[gburd]

Both fixed in #155. I reproduced each as a hard SIGSEGV first, then gated them.

1. rep_get_nsites() pre-open

Program received signal SIGSEGV
#0  __repmgr_get_nsites (...) at ../src/repmgr/repmgr_util.c:520
#1  main () at b.c:10

__repmgr_get_nsites() dereferenced db_rep->region, which is only attached at env open, and the caller's ENV_NOT_CONFIGURED() guard expands to nothing until ENV_OPEN_CALLED is set (src/dbinc/db_int.in:614). Now returns the BDB3672 Nsites unknown before repmgr_start() error the function already had for the analogous case.

One refinement to the report: a plain db_env_create + rep_get_nsites does not crash — rep_handle is fine, it's ->region that is NULL. The trigger is touching any repmgr setting first (e.g. repmgr_set_ack_policy), which allocates DB_REP without attaching its region. That's what the regression test does.

2. __cdsgroup_begin() NULL method slots

Confirmed exactly as reported — 8 of 12 installed, with get_priority, set_priority, set_commit_token, set_txn_lsnp left NULL. The three int-returning slots now use the file's existing __cdsgroup_notsup() helper (DB_OPNOTSUP, "CDS groups do not support %s"), matching the eight already there.

set_txn_lsnp needed different treatment: it is declared void, so it cannot report an error. It nulls its out-parameters instead, which is what the log-path callers already test for.

Test

test/db/null_method_slots.c, 9 checks, in the CI regression-runner step:

  rep_get_nsites(unopened env): ret=22 (Invalid argument)
  cdsgroup get_priority:     ret=95
  cdsgroup set_priority:     ret=95
  cdsgroup set_commit_token: ret=95
  cdsgroup set_txn_lsnp:     rlsnp=(nil) lsnp=(nil)
null_method_slots: 9 checks, 0 failures

It asserts each slot is non-NULL and that calling it returns, and skips case 1 gracefully on a build without repmgr. Reverting either fix makes it report FAIL (rc=139, Segmentation fault), so it has teeth.

Both were reachable from documented public APIs with valid arguments — only the call order or the handle type was unusual. Thanks for the precise report.