Keep the DD and session counters out of module storage (#64) - #66
Open
mgrossmann wants to merge 3 commits into
Open
Keep the DD and session counters out of module storage (#64)#66mgrossmann wants to merge 3 commits into
mgrossmann wants to merge 3 commits into
Conversation
UFSD is link-edited AC(1). Fetched from an APF-authorized library the job
step is already authorized when program fetch runs, so MVS obtains the job
pack area in subpool 252 KEY 0 -- authorized code must not be patchable by
problem-key code. The STC itself runs problem state key 8, so every store
into the module's own storage takes a protection exception.
Startup died on the first one, `++s_ddn_seq` in ufsd_disk_mount_dyn, right
after UFSD047I and before UFSD001I:
PSW AT ENTRY TO ABEND 078D2000 000ADD0A ILC 4 INTC 0004
REGS AT TIME OF ERROR ... R3 000ACC78 R4 00000001 R12 000ADC0A
SPQE 009BF2B0 ... SPID 252 KEY 0 DQE BLK 000A5000 LN 00021000
R3 addresses the fetched module (EPA 000A5B18, extent 000204E8) and the
failing instruction is the ST of `L 3,=A(@v1) / L 4,0(3) / A 4,=F'1' /
ST 4,0(3)` -- R4 holds the incremented 1. `s_sess_serial` in ufsd_sess_open
is the same defect one step further on: it would have abended on the first
client session instead.
Authorizing ourselves through SVC 244 hides it. That sets JSCBAUTH after the
fetch and cannot relabel storage already allocated, so the module stays key 8
and the store goes through -- which is why this only appears on systems that
APF-authorize the LINKLIB, and why removing the APF entry "fixes" it.
Both counters move into UFSD_STC, a main() local and therefore key 8.
ufsd_sess_open reaches it through anchor->server_stc, set before the SSCT is
registered, so no client can arrive before it exists.
tools/check-module-data.py rejects mutable file-scope data and function-local
statics in the AC(1) modules and runs as its own CI job. UFSFMT is exempt:
AC(0) means the job step is never authorized, so it is fetched key 8. The
check covers only what this repo owns; the libc370 members linked into UFSD
were audited by hand for this issue -- per-task state lives in the heap CRT,
the module-resident statics are no-CRT fallbacks.
The same class of defect turns fatal a second way if a module is ever marked
RENT and placed in the key-0, page-protected LPA, so the rule is worth having
regardless of how UFSD is authorized.
ld370 marks a load module RENT and REUS by default; `norent` / `noreus` in project.toml opt out, and none of our modules does. So the two counters were not merely at risk of breaking a future reentrancy claim -- they broke the one the module already carries, and would have failed the same way in the LPA. While in there: state the guard's boundary rather than leaving it implied. It reads [[module]] sources, so client/libufs.c is not covered -- that one is a library, and the storage key of the module it lands in belongs to the consuming program. Scanned by hand for #64: no module-resident data at all. The absent-`ac` default is now noted as deliberately matching mbtconfig.py, so a module that omits the key cannot be skipped by accident.
Open
Running the check against the other mbt v2 projects turned up two parser
faults it cannot hit inside ufsd:
`typedef struct { ... } NAME;` -- ufsd names its structs and typedefs them
separately, httpd and nsf370 do not. The closing declarator was read as a
definition, so ROUTE_POLICY and NSFV_STC were reported as writable data. The
block head is now kept on a stack and the tail dropped only when that head
carried `typedef`; `struct tag { ... } instance;` is still data and still
reported.
Non-C sources were fed to the C parser -- nsf370 lists asm/nsfvsvc.asm in a
module, and comment prose came back as declarations. Sources are filtered to
.c now. Hand-written assembler is not the place for this check anyway: DS/DC
in a CSECT is visible on sight, no parsing needed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #64.
What happens
UFSD is link-edited
AC(1). Fetched from an APF-authorized library the job step is already authorized when program fetch runs, so MVS obtains the job pack area in subpool 252, key 0 — authorized code must not be patchable by problem-key code. The STC runs problem state key 8, so any store into the module's own storage is a protection exception.The three dumps on the issue are identical and land on the first such store,
++s_ddn_seqinufsd_disk_mount_dyn— right afterUFSD047I, beforeUFSD001I READY:R3 addresses the fetched module, R12 matches the function's
BALR 12,0base, and the bytes at0ADCF4areL 3,=A(@V1) / L 4,0(3) / A 4,=F'1' / ST 4,0(3)— theSTis the failing instruction, R4 the incremented 1.s_sess_serialinufsd_sess_openis the same defect one step further on: withs_ddn_seqalone fixed, the first client session would have abended instead.Why the SVC 244 route is unaffected: it sets
JSCBAUTHafter the fetch and cannot relabel storage that is already allocated, so the module stays key 8. That is why the STC works on a stock TK4-/TK5 and only fails when the LINKLIB carries an APF entry.Changes
UFSD_STC— amain()local, hence key 8.ufsd_sess_openreaches it throughanchor->server_stc, which is set before the SSCT is registered, so no client can arrive before it exists.tools/check-module-data.pyrejects mutable file-scope data and function-local statics in theAC(1)modules; own CI job, no toolchain needed. UFSFMT is exempt —AC(0), so the job step is never authorized and the module is fetched key 8.CLAUDE.md, plus the APF section and a troubleshooting row indocs/installation.md.Exposure audit
Beyond the two counters, the generated assembler was scanned for stores through a register loaded from
=A(@Vn)(that is what found these): nothing else in UFSD, UFSDSSIR or UFSDCLNP, nothing inclient/libufs.c(not covered by the guard — it is a library, so the storage key of the module it lands in is the consuming program's business), and nothing in the libc370 members linked intobuild/UFSD— the C library keeps per-task state in the heap-allocated CRT and uses module statics only as no-CRT fallbacks.There is a second reason to want the rule, independent of authorization: ld370 marks a load module RENT and REUS by default (
norent/noreusopt out, and none of our modules does). So these counters were not merely at risk of breaking a future reentrancy claim — they broke the one the module already carries, and would have failed the same way in the LPA. Link attributes are deliberately untouched here.Verification
makeclean,make test-host180/180 assertions pass, lint clean — and the lint flags the pre-fix source when pointed at it./S UFSDwith an APF-authorizedUFSD.LINKLIBreachingUFSD001I READY, plus one client session (HTTPD or FTPD) to exercise the second counter.