Skip to content

Add ESTAE recovery around per-command processing (try()/tryrc()) - #65

Merged
mgrossmann merged 3 commits into
mainfrom
feat/estae-command-recovery-63
Jul 25, 2026
Merged

Add ESTAE recovery around per-command processing (try()/tryrc())#65
mgrossmann merged 3 commits into
mainfrom
feat/estae-command-recovery-63

Conversation

@mgrossmann

Copy link
Copy Markdown
Contributor

Closes #63.

Adds an ESTAE-style recovery boundary around per-command processing so a
single unexpected ABEND aborts only the current command instead of killing the
worker subtask and dropping the client. Defense-in-depth over the
check_dataset_access() / ACEE pre-checks from #61 — a RAKF denial still returns
a clean 550 and never reaches recovery.

What changed

  • ftpd#ses.c — wrap per-command dispatch in try(). try() returns the
    ABEND code and discards the wrapped function's rc, so a thin ftpd_run_command
    wrapper stashes the dispatcher's rc in sess->dispatch_rc (preserving the
    -1→break for QUIT/auth-failure). ftpd_session_recover() runs the cleanup;
    a FTPD_MAX_RECOVER consecutive-recovery guard closes a wedged session cleanly
    (worker survives, serves the next client).
  • ftpd.c — capture server->stc_acee = racf_get_acee() once at init,
    before any worker subtask is attached.
  • ftpd#mvs.c / ftpd#ufs.c — track the in-flight transfer handle
    (cur_file for MVS RETR/STOR/APPE, cur_ufs_file for UFS RETR/STOR); cleared
    before close for idempotency.
  • doc/FTPD_RAKF_SETUP.md — SECURITY INVARIANT: FTPD/USER must stay
    least-privilege (see below).

Recovery handler ordering (deliberate)

The handler runs under try() itself, so a cleanup re-ABEND is contained. Order:

  1. racf_set_acee(stc_acee) — reset the AS-wide ASXBSENV to the STC identity.
  2. unlock(asxb) — release the ASXB ENQ if the ABEND struck inside
    racf_auth()'s lock() critical section (see finding).
  3. WTO decoded ABEND (Sxxx/Uxxxx) + command, then 451 to the client —
    both before the one genuinely risky step.
  4. fclose/ufs_fclose the transfer handle, then close the data connection.

Mechanism (why the handler can use ordinary services)

crent370's try() is SDWA-retry that unwinds the stack back to the try()
frame and resumes in normal task mode
(libc370/src/clib/@@@try.c), so the
if (abrc) branch runs on a valid stack — fclose/racf_set_acee/send/WTO
are all legal — and the heap-allocated session struct survives.

Point-D finding — racf_auth() and the ACEE race

racf_auth(ACEE*, …) (libc370/src/racf/racauth.c) takes the identity
explicitly but plumbs it through the address-space-wide ASXBSENV (RACHECK
samples that field), wrapping set/RACHECK/restore in lock(asxb)/unlock(asxb).
That ENQ serializes concurrent racf_auth() calls, so the primary
authorization decision — FTPD's check_dataset_access() pre-check before every
security-relevant fopen — is race-free.
The residual race is the
unserialized OPEN-path racf_set_acee(sess->acee)/fopen/restore switch, whose
worst case for a pre-checked open is a spurious OPEN failure (availability), not
escalation. Severity: hardening / defense-in-depth. Tracked, verify-first,
in #64 — not fixed here.

Consequence surfaced by this change: lock()/unlock() are TCB-level MVS ENQs
auto-DEQ'd at task termination. Because recovery keeps the TCB alive, an ABEND
inside racf_auth()'s critical section would orphan the AS-wide ASXB ENQ and
stall every session's authorization — hence the defensive unlock(asxb) in the
handler (safe when not held).

SECURITY INVARIANT

Recovery's ASXBSENV reset is fail-closed only while FTPD/USER is
least-privilege
: a concurrent session transiently pulled onto that identity can
only lose authority, never gain it. If FTPD/USER is granted broad dataset
authority the reset degrades to fail-open. Documented as an invariant in
doc/FTPD_RAKF_SETUP.md §5.1.

Scope / documented residuals

Handle tracking is on the transfer paths only. Not instrumented (documented
residuals): metadata fopens (SIZE/LIST/MDTM); the STOR __dsalcf/__dsfree
window (leaks the DD and an empty catalogued dataset); STOR-binary's
record_buffer. total_recover in the operator log (FTPD070E … total=n) makes
any accumulation observable.

Verification

  • Build-verified only: clean -Wall -Werror cc370 build (make modules).
    New operator messages FTPD070E (recovery) and FTPD071E (guard-close)
    checked against existing numbers — no collision.
  • Behavioral verification pending on MVS/CE + TK5 (not yet exercised):
    • trigger a recoverable ABEND → client gets 451, worker stays up, next
      command works, and a subsequent RAKF denial still shows the correct user
      identity (proves ASXBSENV was restored);
    • regression: normal RETR/STOR/LIST/JES unaffected; RAKF denials still produce
      a clean 550 (recovery not triggered on the happy-denial path).

ABEND recovery cannot be meaningfully unit-tested on the host; the runtime proof
above is the acceptance gate.

An unexpected ABEND (S0C4, S806, S213, S913, S130, ...) while handling one
FTP command previously terminated the whole worker subtask and dropped the
client's control connection, abandoning any in-flight resources.

Wrap the per-command dispatch in crent370's try() so a single ABEND aborts
only that command. On ABEND the recovery handler:

  - resets the address-space-wide ASXBSENV to the STC identity (captured
    once at init, before any worker spawns) so the worker never continues
    under a user (or freed) ACEE. FTPD/USER is least-privilege, so a
    concurrent session transiently pulled onto it can only lose authority
    (fail-closed) -- documented as a SECURITY INVARIANT in the RAKF guide;
  - releases the AS-wide ASXB ENQ if the ABEND struck inside racf_auth()'s
    lock() critical section (a surviving task would otherwise orphan it and
    stall every session's authorization);
  - WTOs the decoded ABEND code + command and sends the client
    451 local error -- both before the one step that can re-ABEND;
  - closes the in-flight transfer handle (MVS fclose / UFS ufs_fclose,
    cleared before close for idempotency) and the data connection.

try() discards the wrapped function's return value, so the dispatcher's rc
(which signals loop exit for QUIT/auth-failure) is stashed in the session
and read after try() returns. A consecutive-recovery guard
(FTPD_MAX_RECOVER) closes a wedged session cleanly; the worker survives and
serves the next client. Startup/init ABENDs remain fatal.

In-flight handle tracking (cur_file / cur_ufs_file) covers the transfer
paths only (MVS RETR/STOR/APPE, UFS RETR/STOR). Metadata opens, the STOR
__dsalcf/__dsfree window, and STOR-binary's record_buffer are documented
residuals; total_recover in the operator log makes accumulation observable.

Defense-in-depth over the check_dataset_access()/ACEE pre-checks from #61 --
a RAKF denial still returns a clean 550 and never reaches recovery. The
pre-existing cross-session ASXBSENV race surfaced during review is tracked
separately in #64.

Closes #63
@mgrossmann mgrossmann added priority:medium Normal priority type:feature New functionality labels Jul 22, 2026
Address review of the recovery boundary:

- The recovery unlock(asxb) is ownership-safe without a tracking flag and
  cannot break racf_auth()'s cross-task serialization. unlock() is
  DEQ RET=HAVE (@@lkunlk.c -> @@enqdeq.c: pl.opt=HAVE, SVC 48, returns an
  RC, never ABENDs), and MVS ENQ ownership is per-TCB, so this DEQ can
  only release an ENQ owned by the abending task. Outside racf_auth()'s
  lock window (the common case) the task does not own it and DEQ RET=HAVE
  is a RC=8 no-op -- it can never touch a concurrent worker's lock.
  Comment expanded to record this so it is not re-litigated.

- Note the UFS recovery close as a known limitation: ufs_fclose() is a
  cross-AS request with no timeout in the libufs API, so a dead/hung UFSD
  can block it (the inner try() catches ABENDs, not hangs). The 451 is
  already sent; this is no worse than ufsfree() at session teardown.

- Add a debug-only ABEND injection hook (SITE ABEND / SITE ABEND=LOCK),
  gated behind -DFTPD_DEBUG_ABEND and compiled out of production, to
  exercise both recovery paths on MVS: an ABEND outside any lock window
  (unlock must be a no-op, concurrent racf_auth() undisturbed) and an
  ABEND while holding the ASXB ENQ (recovery must DEQ the orphan).

Refs #63
@mgrossmann

Copy link
Copy Markdown
Contributor Author

Review follow-up: ASXB ENQ ownership-safety (blocker resolved)

Concern: an unconditional unlock(asxb) in recovery could, when the ABEND
lands outside racf_auth()'s lock window (the common case), release a lock
this worker doesn't hold — silently breaking a concurrent worker's mutual
exclusion and manufacturing the very race #64's "hardening" classification
depends on being absent.

Resolution — it's ENQ/DEQ, not a memory word, so that failure mode can't occur:

  1. Mechanism: lock()/unlock() are MVS ENQ/DEQ SVCs, not a memory word.
    @@lk.cENQ(CLIBLOCK, "LOCK.<addr>", HAVE) (SVC 56); @@lkunlk.c
    DEQ(CLIBLOCK, "LOCK.<addr>", HAVE) (SVC 48), SCOPE=STEP. @@enqdeq.c:34-38
    shows the DEQ path sets pl.opt = ENQ_OPT_HAVE (RET=HAVE) and returns
    pl.rc — it returns an RC, never ABENDs.
  2. Currently unconditional — and that is already ownership-safe. MVS ENQ
    ownership is per-TCB; workers are separate TCBs (ATTACH EP=CTHREAD). A
    DEQ RET=HAVE from worker A can only release an ENQ A's TCB owns. Outside
    the window A owns nothing → RC=8 no-op; it cannot touch worker B's ENQ.
    An explicit holds_asxb_lock flag would be redundant (the OS enforces it
    exactly) and, since lock(asxb) is taken inside crent370's racf_auth(),
    an ftpd-level flag couldn't observe that window precisely anyway.
  3. Not counting/recursive. ENQ RET=HAVE grants boolean per-TCB ownership
    (duplicate ENQ → RC=8, no second element; cliblock.h documents lock()
    returning 8 "if you already have the lock"). racf_auth() does one balanced
    lock/unlock per call, so a single DEQ fully releases.

Therefore #64's severity classification stands unchanged: recovery's
unlock(asxb) cannot break racf_auth()'s serialization. The reasoning is now
recorded in the step-2 comment so it isn't re-litigated.

UFS recovery close — known limitation

ufs_fclose() is a cross-AS request with no timeout in the libufs API; a
dead/hung UFSD can block it (the inner try() catches ABENDs, not hangs).
The 451 is already sent so the client isn't left hanging, and this is no worse
than ufsfree() at session teardown, which would also block on a dead UFSD.
Documented in the recovery comment; bounding it needs a libufs timeout that
doesn't exist yet.

Test hook for the pending MVS verification

Added a debug-only ABEND injection, gated behind -DFTPD_DEBUG_ABEND
(compiled out of production):

  • SITE ABEND → ABEND (S0C4) outside any lock window → recovery's
    unlock(asxb) must be a no-op; a concurrent session's racf_auth()
    undisturbed.
  • SITE ABEND=LOCK → acquire the ASXB ENQ, then ABEND holding it →
    recovery must DEQ the orphan; a concurrent session parked in racf_auth()
    then proceeds.

This makes the "A does not hold the lock, B mid-transfer" scenario reproducibly
triggerable on TK5, which it otherwise isn't.

Pre-merge review follow-ups (all debug code remains compiled out unless
-DFTPD_DEBUG_ABEND):

- Document the load-bearing ordering in ftpd_session_recover(): the
  racf_set_acee(STC) reset MUST precede unlock(asxb). Releasing the lock
  first would let a waiting worker capture this session's user ACEE as its
  oldacee and restore it on exit, leaving ASXBSENV dangling once the ACEE
  is freed at racf_logout -- the exact hazard the reset removes. Marked
  "do not reorder" so a future refactor cannot silently swap the lines.

- Add SITE ABEND=XFER: arms the next MVS RETR to ABEND mid-transfer with
  cur_file set and the data connection open, exercising recovery's
  fclose(cur_file) and the clear-before-close idempotency -- the one step
  the earlier SITE ABEND / ABEND=LOCK variants (no open file) never hit.
  Recorded in-code that S0C4 is a clean program check: the messier
  realistic cases (S013/S001/S878) are not reproduced, so the hook proves
  the recovery mechanism, not the hardest DCB/buffer states.

- Debug-hook hygiene: the injection now requires sess->authenticated (SITE
  is already post-auth; the explicit guard prevents any pre-auth DoS in
  debug builds), the entire intercept and its include sit inside the
  #ifdef, HELP/FEAT advertise nothing, and the production build (flag off)
  is behaviourally unchanged.

Refs #63
@mgrossmann

Copy link
Copy Markdown
Contributor Author

Pre-merge follow-ups (items 1–3)

1. Load-bearing ordering documented. ftpd_session_recover() now carries an explicit do-not-reorder note: racf_set_acee(STC) must precede unlock(asxb). If the lock were released first, a waiting worker would capture this session's user ACEE as its oldacee and restore it on exit, leaving ASXBSENV dangling once the ACEE is freed at racf_logout — the exact hazard the reset removes.

2. SITE ABEND=XFER added. Arms the next MVS RETR to ABEND mid-transfer with cur_file set and the data connection open — exercising the one "genuinely risky" step (fclose(cur_file)) and the clear-before-close idempotency that SITE ABEND/ABEND=LOCK (no open file) never reach. Acceptance: 451 to client, fclose runs exactly once (no double-close), data socket closed, worker survives, next command works.

What the hook does NOT prove: S0C4 is a clean program check at a defined instruction. The realistic messy cases (S013/S001/S878) leave dirtier DCB/buffer state and are not reproduced. The hook validates the recovery mechanism, not the hardest states — green hook runs are not full coverage.

3. Debug-hook hygiene (all under -DFTPD_DEBUG_ABEND, compiled out of production):

  • injection requires sess->authenticated (SITE is already post-auth; explicit guard prevents any pre-auth DoS in debug builds);
  • the entire intercept and its cliblock.h include sit inside the #ifdef — no dispatch entry leaks into production;
  • HELP/FEAT advertise nothing in either build;
  • production build (flag off) is behaviourally unchanged — every debug symbol (debug_abend_xfer, the SITE ABEND block, the RETR injection) is #ifdef-guarded. Verified both builds compile clean under cc370.

On-MVS behavioural verification (now runnable via the hook) remains the acceptance gate.

@mgrossmann
mgrossmann merged commit 7838a2b into main Jul 25, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority:medium Normal priority type:feature New functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FTPD: ESTAE-style recovery around per-command processing (try()/tryrc()) — an ABEND must not kill the worker

1 participant