Skip Menu |
 

From: ghudson@mit.edu
Date: Thu, 18 Jun 2020 21:18:31 -0400
Subject: KDC and kadmind fork with DB open, breaking LMDB KDB module
To: rt@krbdev.mit.edu
lmdb.h says "Use an MDB_env* in the process which opened it, not after fork()."  The reason is that LMDB tracks stale reader entries by pid.  (Rather than using kill() to detect if the locking pid corresponds to an active process, it creates a POSIX one-byte range lock using the pid as an offset when the DB is opened, and tests for the existence of the lock.  A fork()/exit() causes this lock to be dropped, even though the environment fd is still open.)

krb5kdc and kadmind both open the database before daemonizing, so they violate this restriction.  The consequence is that their reader entries can be stolen and invalidated by other processes (including each other).  The bereft process will then receive MDB_BAD_RSLOT ("Invalid reuse of reader locktable slot") errors when trying to renew its read transaction.

This bug does not manifest in the automated tests, because we mostly start krb5kdc and kadmind with the nofork option set.  It also may not manifest if krb5kdc and kadmind are started simultaneously, as they may both claim their reader transaction slots before either one of them forks and exits.

My manual process for reproducing the bug is:

  make testrealm
  pkill krb5kdc
  pkill kadmind
  krb5kdc
  kadmind
  kinit user

See also: https://mailman.mit.edu/pipermail/kerberos/2020-June/022507.html
From: ghudson@mit.edu
Subject: git commit

Avoid using LMDB environments across forks

In krb5kdc and kadmind, reinitialize the DB state after daemonizing,
to prevent using an LMDB environment in a different process than it
was created. Otherwise the daemon's reader table slot appears to be
stale and can be claimed by another process.

In kadmind, this change means that global_server_handle changes value
after the loop setup. Add an extra level of pointer indirection so
that the handle passed to the loop remains valid.

kdb_init_hist() is now called twice by kadmind. Change it to avoid
leaking hist_princ on the second invocation.

https://github.com/krb5/krb5/commit/38b98a14433b8858a3ca5979a0afa194df0df1e9
Author: Greg Hudson <ghudson@mit.edu>
Commit: 38b98a14433b8858a3ca5979a0afa194df0df1e9
Branch: master
src/kadmin/server/misc.c | 4 ++--
src/kadmin/server/ovsec_kadmd.c | 15 +++++++++++++--
src/kadmin/server/schpw.c | 4 ++--
src/kdc/main.c | 11 +++++++----
src/lib/kadm5/srv/server_kdb.c | 2 ++
5 files changed, 26 insertions(+), 10 deletions(-)
Subject: git commit
From: ghudson@mit.edu
Download (untitled) / with headers
text/plain 1.1KiB

Avoid using LMDB environments across forks

In krb5kdc and kadmind, reinitialize the DB state after daemonizing,
to prevent using an LMDB environment in a different process than it
was created. Otherwise the daemon's reader table slot appears to be
stale and can be claimed by another process.

In kadmind, this change means that global_server_handle changes value
after the loop setup. Add an extra level of pointer indirection so
that the handle passed to the loop remains valid.

kdb_init_hist() is now called twice by kadmind. Change it to avoid
leaking hist_princ on the second invocation.

(cherry picked from commit 38b98a14433b8858a3ca5979a0afa194df0df1e9)

https://github.com/krb5/krb5/commit/146245e98795757effee893f9098f8e73131a902
Author: Greg Hudson <ghudson@mit.edu>
Commit: 146245e98795757effee893f9098f8e73131a902
Branch: krb5-1.18
src/kadmin/server/misc.c | 4 ++--
src/kadmin/server/ovsec_kadmd.c | 15 +++++++++++++--
src/kadmin/server/schpw.c | 4 ++--
src/kdc/main.c | 11 +++++++----
src/lib/kadm5/srv/server_kdb.c | 2 ++
5 files changed, 26 insertions(+), 10 deletions(-)
From: ghudson@mit.edu
Subject: git commit
Download (untitled) / with headers
text/plain 1.1KiB

Avoid using LMDB environments across forks

In krb5kdc and kadmind, reinitialize the DB state after daemonizing,
to prevent using an LMDB environment in a different process than it
was created. Otherwise the daemon's reader table slot appears to be
stale and can be claimed by another process.

In kadmind, this change means that global_server_handle changes value
after the loop setup. Add an extra level of pointer indirection so
that the handle passed to the loop remains valid.

kdb_init_hist() is now called twice by kadmind. Change it to avoid
leaking hist_princ on the second invocation.

(cherry picked from commit 38b98a14433b8858a3ca5979a0afa194df0df1e9)

https://github.com/krb5/krb5/commit/75ae7431dbefc4b2ec082a4cfe3f65749fde0fda
Author: Greg Hudson <ghudson@mit.edu>
Commit: 75ae7431dbefc4b2ec082a4cfe3f65749fde0fda
Branch: krb5-1.17
src/kadmin/server/misc.c | 4 ++--
src/kadmin/server/ovsec_kadmd.c | 15 +++++++++++++--
src/kadmin/server/schpw.c | 4 ++--
src/kdc/main.c | 11 +++++++----
src/lib/kadm5/srv/server_kdb.c | 2 ++
5 files changed, 26 insertions(+), 10 deletions(-)