Keep the DD and session counters out of module storage (#64)

.github/workflows/build.yml

@@ -8,3 +8,21 @@ on:
 jobs:
   build:
     uses: mvslovers/mbt/.github/workflows/build.yml@main
+
+  # An AC(1) module fetched from an APF-authorized library is loaded into
+  # key-0 storage, so a key-8 store into its own statics abends S0C4 (#64).
+  # The build cannot see that -- this can.
+  module-data:
+    name: No writable data in AC(1) modules
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v6
+
+      - name: Set up Python
+        uses: actions/setup-python@v6
+        with:
+          python-version: "3.12"
+
+      - name: Check module data
+        run: python3 tools/check-module-data.py

CLAUDE.md

@@ -47,6 +47,14 @@ These are hard-won from implementation. Violating them causes abends:
 6. **4K buffer pattern:** `do_fread`/`do_fwrite` use heap staging buffers.
    `ufsd_dispatch` copies to/from CSA UFSBUF in key-0. The staging pointer is
    passed via `resp_data[4..7]`.
+7. **No writable statics in an AC(1) module.** UFSD/UFSDSSIR/UFSDCLNP are
+   link-edited AC(1); from an APF-authorized library program fetch takes the
+   job pack area in subpool 252 **key 0**, while the STC runs key 8 — a store
+   into a C static or non-const global then abends S0C4 (#64). SVC 244 only
+   sets `JSCBAUTH` afterwards and changes nothing about the storage key. Put
+   the value in `UFSD_STC` (a `main()` local, key 8, reachable through
+   `anchor->server_stc`), on the heap, or in CSA behind the key-0 window.
+   `tools/check-module-data.py` enforces this and runs in CI.
 
 ## Build
 
@@ -93,3 +101,4 @@ Client: `client/libufs.c` (stub library — includes ufs_stat), `client/libufsts
 - Dispatch functions (ufsd#fil.c) NEVER write CSA directly — use resp_data[]
 - 4K data transfers use staging buffers (heap) — ufsd_dispatch copies to/from CSA
 - UFSFILE in libufs: ~8K per handle (4K rbuf + 4K wbuf) — document if this grows
+- No mutable `static`/global at any scope in an AC(1) module — see constraint 7

docs/installation.md

@@ -89,6 +89,15 @@ The clean alternative is to add `UFSD.<vrm>.LINKLIB` to the APF list in
 IPL, and that the library name carries the version — so this is one IPL per
 release, not one IPL ever.
 
+The two routes are not quite the same underneath. UFSD is link-edited AC(1),
+so from an APF-authorized library the job step is already authorized when
+program fetch runs, and MVS obtains the job pack area in **key 0** — the
+module's own storage is then read-only for the STC, which runs key 8. Via
+SVC 244 the module is fetched key 8 and `JSCBAUTH` is set afterwards, which
+does not relabel storage already allocated. UFSD therefore keeps no writable
+data in module storage; releases up to 1.2.0 did, and abend S0C4 during
+startup on the APF route (issue #64).
+
 If neither applies to your system, resolve it before step 5: an install that
 completes cleanly will still not start.
 
@@ -420,6 +429,7 @@ linkage convention are documented in
 | `S806` (module not found) at `/S UFSD` | `STEPLIB` in the procedure does not name the LINKLIB the APPLY wrote to |
 | `/S UFSD` rejected — procedure not found | Procedure not copied into a PROCLIB in the started-task concatenation |
 | `UFSD091E APF SETUP FAILED` | No RAKF (so no SVC 244) and no APF entry — see step 2 |
+| `S0C4` right after `UFSD047I`, only with an APF-authorized LINKLIB | Release 1.2.0 or earlier: the module is fetched key 0 there, and a static counter is written key 8 (issue #64). Upgrade, or drop the APF entry and let SVC 244 do it |
 | `UFSD061E PARMLIB … NOT FOUND` | `D=`/`M=` wrong, or the member is in a PARMLIB the procedure does not name. On TK5 this is usually `SYS2.PARMLIB` not existing — step 6 |
 | Mount fails / `UFSD124E` superblock validation | The `ROOT`/`MOUNT` dataset does not exist or is not UFS-formatted — step 7 |
 | `S106` at start on a freshly installed library | The XMIT was uploaded in text mode. Re-upload in **binary** and re-run the install job |

include/ufsd.h

@@ -229,6 +229,13 @@ struct ufsd_stc {
     /* #52: where each disk hangs in the tree, same indexing as
     ** disks[].  Filled at mount time, read per directory entry. */
     UFSD_MOUNTPT    mountpt[UFSD_MAX_DISKS];
+    /* #64: counters that must not live in module storage.  UFSD is
+    ** linked AC(1); fetched from an APF-authorized library it lands in
+    ** subpool 252 KEY 0, and the STC runs problem state key 8 -- so a
+    ** store into a C static abends S0C4.  This block is a main() local
+    ** (key 8) and is reachable from anywhere via anchor->server_stc. */
+    unsigned        ddn_seq;        /* DYNALLOC DD name sequence    */
+    unsigned        sess_serial;    /* session token serial         */
 };
 
 /* ============================================================

src/ufsd#ini.c

@@ -35,8 +35,9 @@ struct ufsboot_hdr {
 };                              /* 08                                   */
 #define UFSD_DISK_TYPE_UFS  2
 
-/* DD name sequence counter for DYNALLOC */
-static unsigned s_ddn_seq = 0;
+/* The DD name sequence counter lives in UFSD_STC (stc->ddn_seq), not in a
+** C static: with AC(1) from an APF-authorized library the module is fetched
+** into key-0 storage, where a key-8 store abends S0C4 (#64). */
 
 /* Forward declarations */
 static UFSD_DISK *open_disk(const char *ddname);
@@ -451,7 +452,7 @@ ufsd_disk_mount_dyn(UFSD_STC *stc, const char *dsname,
     }
 
     /* Generate DD name: UFD00001, UFD00002, ... */
-    sprintf(ddname, "UFD%05u", ++s_ddn_seq);
+    sprintf(ddname, "UFD%05u", ++stc->ddn_seq);
 
     /* DYNALLOC: DISP=OLD for RW (exclusive), DISP=SHR for RO */
     if (ufsd_dynalloc(ddname, dsname, mode) != 0)

src/ufsd#ses.c

@@ -5,7 +5,7 @@
 **
 ** Token scheme: ((slot_index + 1) << 16) | (serial & 0xFFFF)
 **   - Slot 0 produces tokens 0x0001xxxx
-**   - serial is a static counter, wraps at 0xFFFF
+**   - serial counts up in UFSD_STC (stc->sess_serial), wraps at 0xFFFF
 **   - Token 0 is never issued (slot+1 ensures non-zero high word)
 **
 ** Memory layout:
@@ -32,9 +32,13 @@
 #include <cvt.h>
 #include <ihaasvt.h>
 
-/* Static serial counter.  Accessed only from the STC (problem state,
-** single-threaded in Phase 1).  NOT in CSA; no key-0 window needed. */
-static unsigned s_sess_serial = 0;
+/* The serial counter lives in UFSD_STC (stc->sess_serial), reached through
+** anchor->server_stc.  It must not be a C static: UFSD is linked AC(1), and
+** fetched from an APF-authorized library the module lands in subpool 252
+** KEY 0 while the STC runs problem state key 8 -- a store into module
+** storage then abends S0C4 (#64).  UFSD_STC is a main() local, hence key 8.
+** Accessed only from the STC (problem state, single-threaded in Phase 1);
+** it is not in CSA, so no key-0 window is needed. */
 
 /* ============================================================
 ** ufsd_sess_init
@@ -174,10 +178,17 @@ ufsd_sess_open(UFSD_ANCHOR *anchor, UFSREQ *req, unsigned *out_token)
     UFSD_SESSION *sess;
     unsigned      token;
     UFSD_UFS     *ufs;
+    UFSD_STC     *stc;
     int           j;
 
     if (!anchor || !anchor->sessions || !out_token) return UFSD_RC_CORRUPT;
 
+    /* Set at startup, before the SSI router is registered -- so by the time
+    ** a client can reach us it is there.  Without it there is no writable
+    ** home for the serial (see the note at the top of this file). */
+    stc = (UFSD_STC *)anchor->server_stc;
+    if (!stc) return UFSD_RC_CORRUPT;
+
     *out_token = 0;
 
     /* Find the first inactive slot */
@@ -191,8 +202,8 @@ ufsd_sess_open(UFSD_ANCHOR *anchor, UFSREQ *req, unsigned *out_token)
     if (slot >= anchor->max_sessions) return UFSD_RC_NOREQ;
 
     /* Generate token */
-    s_sess_serial++;
-    token = ((slot + 1U) << 16) | (s_sess_serial & 0xFFFFU);
+    stc->sess_serial++;
+    token = ((slot + 1U) << 16) | (stc->sess_serial & 0xFFFFU);
 
     /* Allocate per-session UFS handle */
     ufs = (UFSD_UFS *)calloc(1, sizeof(UFSD_UFS));

tools/check-module-data.py

@@ -0,0 +1,210 @@
+#!/usr/bin/env python3
+"""Reject writable file-scope data in the AC(1) load modules.
+
+Why this exists (issue #64)
+---------------------------
+UFSD, UFSDSSIR and UFSDCLNP are link-edited AC(1).  Fetched from an
+APF-authorized library, the job step is authorized before program fetch runs,
+and MVS then obtains the job pack area in subpool 252 with **storage key 0**
+so that problem-key code cannot patch authorized code.  The STC itself runs
+problem state key 8.  Every store into the module's own storage -- i.e. every
+write to a C static or a non-const global -- therefore takes a protection
+exception:
+
+    PSW AT ENTRY TO ABEND  078D2000 000ADD0A   ILC 4  INTC 0004
+    SPQE ... SPID 252  KEY 0   (DQE covers the fetched module)
+
+Without APF the same module is fetched key 8 and the store goes through
+unnoticed, which is why this only shows up on hardened systems.  Authorizing
+ourselves later via SVC 244 does not change it either way: that sets JSCBAUTH,
+it cannot relabel storage that program fetch already allocated.
+
+There is a second reason, independent of authorization: ld370 marks a load
+module RENT and REUS unless the module opts out with `norent` / `noreus`, and
+none of ours does.  Writable module data breaks that promise outright, and
+would fail the same way in the (key 0, page-protected) LPA.
+
+So: no mutable file-scope data in an AC(1) module.  Put the counter in
+UFSD_STC (a main() local, key 8, reachable through anchor->server_stc), on the
+heap, or in CSA behind the usual key-0 window.
+
+UFSFMT is exempt from the key-0 argument: AC(0) means the job step is never
+authorized, so MVS fetches it key 8.  (An absent `ac` counts as 0, matching
+mbt -- see mbtconfig.py, `mod.get("ac", 0)`.)
+
+Scope and limits
+----------------
+Only `[[module]]` sources are checked, so `client/libufs.c` is not: it is a
+library, and the key of the module it ends up in is the consuming program's
+business.  Scanned by hand for #64 -- no module-resident data at all.
+
+libc370 is out of reach too (different repo).  Audited by hand for #64: per-
+task state lives in the heap-allocated CRT, the module-resident statics are
+no-CRT fallbacks only.
+
+The precise cross-check for any of this is to compile with `cc370 -S` and look
+for a store through a register loaded from `=A(@Vn)` -- that is what found the
+two offenders in the first place.
+
+Usage: tools/check-module-data.py [project.toml]
+"""
+
+import glob
+import os
+import re
+import sys
+
+try:
+    import tomllib
+except ImportError:                                     # Python < 3.11
+    sys.exit("check-module-data: needs Python 3.11+ (tomllib)")
+
+
+def strip_noise(src):
+    """Blank out comments, string/char literals and preprocessor lines.
+
+    Newlines are preserved so reported line numbers stay usable.
+    """
+    out = []
+    i, n = 0, len(src)
+    while i < n:
+        c = src[i]
+        if c == '/' and src[i:i + 2] == '/*':
+            j = src.find('*/', i + 2)
+            j = n if j < 0 else j + 2
+            out.append(''.join(ch if ch == '\n' else ' ' for ch in src[i:j]))
+            i = j
+            continue
+        if c == '/' and src[i:i + 2] == '//':
+            j = src.find('\n', i)
+            j = n if j < 0 else j
+            out.append(' ' * (j - i))
+            i = j
+            continue
+        if c in '"\'':
+            quote, j = c, i + 1
+            while j < n and src[j] != quote:
+                j += 2 if src[j] == '\\' else 1
+            out.append('""' + ' ' * max(0, j - i - 1))
+            i = j + 1
+            continue
+        out.append(c)
+        i += 1
+    text = ''.join(out)
+    return '\n'.join('' if l.lstrip().startswith('#') else l
+                     for l in text.split('\n'))
+
+
+def declarations(src):
+    """Yield (line, text, depth) for every statement, file scope and inside
+    function bodies alike -- a function-local `static` is module storage too.
+
+    A '{' right after '=' or ',' opens an initializer, not a block.  The
+    declarator that closes a `typedef struct { ... } NAME;` is a type name, not
+    data -- but `struct tag { ... } instance;` is data, so only typedef tails
+    are dropped.
+    """
+    depth, init, buf, line, start = 0, 0, '', 1, 1
+    heads, typetail = [], False
+    for ch in src:
+        if ch == '\n':
+            line += 1
+        if ch == '{':
+            if init or buf.rstrip().endswith(('=', ',')):
+                init += 1               # aggregate initializer, keep reading
+                buf += ' '
+                continue
+            depth += 1
+            heads.append(' '.join(buf.split()))
+            buf, start = '', line
+            continue
+        if ch == '}':
+            if init:
+                init -= 1
+                buf += ' '
+                continue
+            depth = max(0, depth - 1)
+            typetail = bool(re.search(r'\btypedef\b',
+                                      heads.pop() if heads else ''))
+            buf, start = '', line
+            continue
+        if ch == ';' and not init:
+            head = ' '.join(buf.split())
+            if head and not typetail:
+                yield start, head, depth
+            typetail = False
+            buf, start = '', line
+            continue
+        if not buf.strip():
+            start = line
+        buf += ch
+
+
+IS_FUNC = re.compile(r'\([^)]*\)\s*$')
+IS_FUNC_PTR = re.compile(r'\(\s*\*')
+SKIPPABLE = re.compile(r'\b(typedef|extern)\b')
+IS_CONST = re.compile(r'\bconst\b')
+IS_TAG_ONLY = re.compile(r'^(struct|union|enum)\s+\w+$')
+
+
+def mutable(head, depth):
+    if SKIPPABLE.search(head) or IS_CONST.search(head):
+        return False
+    if depth and not head.startswith('static'):
+        return False        # an ordinary local lives on the stack
+    if IS_TAG_ONLY.match(head):
+        return False
+    if IS_FUNC.search(head) and not IS_FUNC_PTR.search(head):
+        return False        # prototype or definition head
+    return bool(re.search(r'\w', head))
+
+
+def sources_of(module, root):
+    """The module's C sources.  Hand-written assembler is not parsed here --
+    its storage is explicit, and `DS`/`DC` in a CSECT is visible on sight."""
+    files = []
+    for pattern in module.get('sources', []):
+        files += glob.glob(os.path.join(root, pattern))
+    dropped = set()
+    for pattern in module.get('exclude', []):
+        dropped |= set(glob.glob(os.path.join(root, pattern)))
+    return sorted(f for f in set(files) - dropped if f.endswith('.c'))
+
+
+def main(argv):
+    toml = argv[1] if len(argv) > 1 else 'project.toml'
+    root = os.path.dirname(os.path.abspath(toml)) or '.'
+    with open(toml, 'rb') as fh:
+        project = tomllib.load(fh)
+
+    findings = []
+    checked = 0
+    for module in project.get('module', []):
+        if module.get('ac', 0) != 1:
+            continue
+        for path in sources_of(module, root):
+            checked += 1
+            src = strip_noise(open(path, encoding='utf-8',
+                                   errors='replace').read())
+            for line, head, depth in declarations(src):
+                if mutable(head, depth):
+                    findings.append((module['name'],
+                                     os.path.relpath(path, root), line, head))
+
+    if not findings:
+        print(f"check-module-data: {checked} sources clean "
+              f"(no writable file-scope data in AC(1) modules)")
+        return 0
+
+    print("check-module-data: writable file-scope data in an AC(1) module\n")
+    for name, path, line, head in findings:
+        print(f"  {path}:{line}: {head[:100]}   [{name}]")
+    print("\nFetched from an APF-authorized library these modules land in "
+          "key-0 storage;\na key-8 store into them abends S0C4 (issue #64). "
+          "Move the value into\nUFSD_STC, onto the heap, or into CSA behind "
+          "a key-0 window -- or make it const.")
+    return 1
+
+
+if __name__ == '__main__':
+    sys.exit(main(sys.argv))