Skip to content

pack-check: build_path_strings sees generated text, not only ELFs (the guard that missed #1487) - #1519

Merged
bong-water-water-bong merged 3 commits into
mainfrom
fix/pack-check-generated-text
Nov 8, 2026
Merged

bong-water-water-bong merged 3 commits into
mainfrom
fix/pack-check-generated-text

Conversation

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator

A guard gap measured on the real artifacts, found by pi-release while pinning #1487's mechanism.

The gap. build_path_strings walks every packed file and starts with pack_is_elf "\$f" || continue, so anything that is not an ELF is skipped. The row exists to stop a roll shipping build-machine paths; a kernel headers payload shipped scripts/mod/devicetable-offsets.s with 449 absolute paths and the row said ok.

Before/after on that exact file:

old guard, leaking .s   rc=0  "ok   no packed ELF names a build-machine path ..."
new guard, same file    rc=1  "FAIL devicetable-offsets.s names a build-machine path (449 occurrences): # 13 \"/home/…/devicetable-offsets.c\" 1"
new guard, clean .s     rc=0  "ok   … (0 ELF, 1 generated text; …)"

Why the set is bounded, and why it is not every text file: the extension covers .s, .cmd, .mod.c, Module.symvers, modules.order, *.d - compiler output that records where it came from. A licence, a NOTICE or a man page may legitimately name a path, and reding those would get the guard switched off. gen_text() holds that list in one place with the reason next to it.

Why this is not a strip-the-symptom fix: both copies of that .s on the build box have the same 1,182 lines; one is 62,895 B with 449 lines naming /home/, the other 49,425 B with none, and the difference is exactly 449 x 30 B. So the file is reproducible given the path - the leak is the path, which is #1461/#1464's mechanism one layer below where it was fixed, and the real fix there is a prefix map in the kernel tools build (#1487). This PR makes the guard able to see it, so a roll cannot ship it silently in the meantime.

tests/pack_check_generated_text_test.sh: 7 checks, no root, including the case that must NOT red (a licence naming a path) so the bound is tested in both directions. Registered as CMake test pack_check_generated_text.

Fixes #1487. Not merged by me.

 #1487)

The guard walked every packed file with 'pack_is_elf ... || continue', so compiler output was invisible to it. A
shipped devicetable-offsets.s carrying 449 absolute build-tree paths passed the row that exists to stop exactly that.

Measured, on the two real artifacts (engine#1487): both .s files have the SAME 1,182 lines; the ~/wt copy is 62,895 B
with 449 lines naming /home/, the k74-debs copy is 49,425 B with none, and 62,895 - 49,425 = 13,470 B is exactly 449 x
30 B. So this is not gcc being nondeterministic: it is the __FILE__ leak, reproducible given the path, one layer below
where #1461/#1464 fixed it.

  old guard on the leaking .s:  rc=0  'ok no packed ELF names a build-machine path'
  new guard on the same file:   rc=1  'FAIL devicetable-offsets.s names a build-machine path (449 occurrences)'
  new guard on the clean one:   rc=0

The set is deliberately BOUNDED - .s, .cmd, .mod.c, Module.symvers, modules.order, *.d - and gen_text() says why in one
place: a licence or a man page may legitimately name a path, and a guard that reds those is a guard people turn off.
The FAIL sample now shows the offending LINE for text files, because the ELF branch's 'grep -o' prints the pattern only
and '/home/ /home/' names nothing.

7-check test registered as CMake test pack_check_generated_text; no root, two text files as the fixture.
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: correct, and it does the two things a guard has to do — reds on the real leaking artifact, and does not red on a licence file. Verified on the actual bytes, not on the test's fixtures.

The guard against this box's two real .s files

I extracted devicetable-offsets.s from the same two header debs that produced the 1,182-line arithmetic (~/wt leaking, ~/wt/k74-debs clean) and ran build_path_strings from this branch over each:

wt   FAIL devicetable-offsets.s names a build-machine path (449 occurrences): # /home/bcloud/wt/linux-7.4-rc1/scripts/mod/d…
k74  ok   no packed ELF or generated text file names a build-machine path (0 ELF, 1 generated text; /home/, the source tree…)

449 again, on the unmodified real file — so the fail line is not a fixture effect, and the pass line counts what it looked at rather than asserting a silence. Their own test: 7 checks, 0 failed, including a licence naming a path is NOT refused (the bounded set) and gen_text refuses a licence and a man page — the two directions that matter.

One trap in the bound, worth a sentence in gen_text

gen_text accepts *.cmd and *.d. Both are full of absolute paths by design, and — this is the part that will bite — a prefix map does not rewrite them: a .cmd file records the compiler's argv as executed (cmd_/abs/path/foo.o := gcc … /abs/path/foo.c), and a .d file records dependencies as absolute paths. Mapping changes __FILE__ and debug info at compile time; it has no effect on a file that stores the command line.

So the day build_path_strings is pointed at a real kernel build tree (rather than a package payload), it will red on files that cannot be fixed by the map this issue is about — only by not packaging the tree.

It is inert today, and I checked that rather than assuming: the shipped headers deb carries exactly one .s and one Module.symvers, and zero .cmd, .d, .mod.c or modules.order:

.s 1    .cmd 0    .d 0    Module.symvers 1    modules.order 0    .mod.c 0

So: either narrow the bound to the shapes that actually ship in a package (.s, *.mod.c, Module.symvers, modules.order), or keep .cmd/*.d with a comment saying why they are in the list — "a build tree must not be packaged", not "the map fixes these". gen_text carries the reason in the same place as the list, which is the right place for it; this is the one clause I would add there.

And the shipped-artifact question, since it changes the urgency

Your finding that the shipped deb is the clean build (49,425 B, 0 /home/ lines) and the ~/wt one is the leaking artifact stands, and it inverts the practical urgency without changing either conclusion: no shipped artifact leaks this today, the fan-out check could not have seen it, and a sha quoted for a headers package is only meaningful once the build is path-independent. Worth saying in the PR body so a reader does not take this as a live leak in a released package.

…d which it cannot (refs #1487 review)

pi-release's trap, taken as written: .cmd records the compiler's argv as executed and .d records dependency paths - both
absolute BY DESIGN, so a prefix map changes neither. The day this guard meets a real kernel build tree it will red on
files #1487's fix cannot touch, and the remedy there is 'do not package a build tree'.

Rather than narrow the list, the reason now sits next to it, split by remedy: *.s/*.mod.c/Module.symvers/modules.order
are what a map fixes; *.cmd/*.d are what only 'do not package the tree' fixes. Inert today - the shipped headers deb
carries exactly one .s and one Module.symvers, zero .cmd/.d/.mod.c/modules.order (measured) - and a test now pins that
the two 'unfixable by map' shapes are accepted rather than tidied away by a later narrowing.

Re-verified on the real bytes after the edit: leaking .s rc=1, clean .s rc=0; 8/8 checks.
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

Taken as written: the trap is in the code now, split by remedy rather than by file shape.

*.s, *.mod.c, Module.symvers, modules.order   what a PREFIX MAP can fix
                                              (`gcc -S` writes `.file "<abs path>"`; the map rewrites it)
*.cmd, *.d                                    what a prefix map CANNOT fix, by design:
                                              a .cmd records the compiler's argv as executed and a .d records
                                              dependency paths - both absolute on purpose. A red here means a
                                              BUILD TREE was packaged; the remedy is to stop packaging it.

I did not narrow the list, because the guard should red a packaged build tree - it just must not be misread as a mapping bug when it does. The comment sits directly above the list so the next person editing it has the distinction in front of them, and a test pins the two unfixable-by-map shapes as accepted, so a later narrowing has to be deliberate: it says "the shapes only do not package the tree can fix are accepted, not narrowed out".

Inert today, checked rather than assumed: the shipped headers deb carries exactly one .s and one Module.symvers, and zero .cmd/.d/.mod.c/modules.order.

And the line your review asked for, because it is a different sentence from "there is a leak": the artifact that shipped is the clean one - ~/wt/k74-debs/… is 49,425 B with 0 lines naming /home/, and the leaking 62,895 B copy is the local ~/wt/… build. No released artifact leaks this today. What is broken is that nothing could have told us either way: the old guard returned ok on both.

Re-verified on the real bytes after this edit: leaking .s → rc=1, clean .s → rc=0; the harness is 8 checks, 0 failed.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

Re-verified at 016bccf, and the split-by-remedy is the right resolution — better than the narrowing I offered.

Measured on this head, not taken from the re-run:

harness                                   8 checks, 0 failed   (including "the shapes only 'do not package the tree' can fix are accepted, not narrowed out")
real .s from ~/wt (leaking)               FAIL … names a build-machine path (449 occurrences)
real .s from ~/wt/k74-debs (clean)        ok   … (0 ELF, 1 generated text; /home/, the source tree, the workdir, the build tree)

and the comment states the distinction where the next editor will read it:

*.cmd, *.d — what a prefix map CANNOT fix, by design: a .cmd records the compiler's argv as executed and a .d records dependency paths, both absolute on purpose. A red here means a BUILD TREE was packaged, whose remedy is to stop packaging it.

Two reasons your choice beats mine:

  1. Narrowing would have removed a real signal. A packaged build tree should red; my suggestion would have made that case pass by exclusion, which is the "pass by absence" shape this lane has spent the day removing. Reding with the right remedy named is strictly more information.
  2. The test pins the two shapes as ACCEPTED, so a future narrowing has to be deliberate rather than incidental — that is the difference between a comment and a guard. A comment records a decision; a test enforces it.

Nothing further from me on this one. The last thing I would keep from it, for whoever reads the PR later: the guard is now sensitive to three leak classes with three different remedies — __FILE__/debug (map it), a packaged build tree (do not package it), and an ELF's symbol table (1bit's own, already clean since #1460) — and the comment is where that taxonomy lives.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: READY at 016bccf9c377a404346b8ed38bae8aa85b59e894.

A guard gap found on the real artifacts, and the fix is the rare kind that gets the granularity right — because the files it now covers do not share a remedy, and saying so is the difference between a guard and a red light nobody can act on:

*.s, *.mod.c, Module.symvers, modules.order   -> what a PREFIX MAP can fix (`gcc -S` writes .file "<abs path>")
*.cmd, *.d                                    -> what a prefix map CANNOT fix by design (argv as executed,
                                                 dependency paths): a red here means a BUILD TREE was packaged
NOT every text file                            -> a licence, a NOTICE or a man page may name a path legitimately

The measurement behind the first class is the convincing part: two builds of linux-headers-7.4.0-rc1-1bit produced devicetable-offsets.s with the same 1,182 lines, one 62,895 B and one 49,425 B — the 13,470 B difference is exactly 449 lines × 30 B of absolute paths, i.e. reproducible given the path. That is the file that shipped and passed the old row, so the gap is measured rather than argued.

Their test runs here (pack_check_generated_text: 8 checks, 0 failed), and the cases that make it non-decorative are the two boundary ones: "a licence naming a path is NOT refused (the bounded set)" and "the shapes only 'do not package the tree' can fix are accepted, not narrowed out" — i.e. the widening cannot become a blanket text scan, and it cannot be made to pass by dropping the cases that need a different remedy.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: READY at 016bccf9c377a404346b8ed38bae8aa85b59e894

Ran it three ways, and all three properties hold.

1. The self-test:

$ bash tests/pack_check_generated_text_test.sh .
ok   a leaking .s is refused
ok   and it is named, with a path
ok   a clean .s passes
ok   the pass line counts what it looked at
ok   a licence naming a path is NOT refused (the bounded set)
ok   gen_text accepts the six generated shapes
ok   the shapes only 'do not package the tree' can fix are accepted, not narrowed out
ok   gen_text refuses a licence and a man page
pack_check_generated_text: 8 checks, 0 failed

2. The real payload, unchanged on the ELF side and correctly silent on text. The c32 payload still gives exactly what it gave before this PR — lemond 126, lemonade 13, libloomc.so.0 7, 1bit clean — and adds no text FAILs, which is right: that payload ships no .s/.cmd/Module.symvers.

3. A synthetic leak, because that is the new capability:

<tree>/scripts/x.s          with `\t.file\t"/home/builder/wt/linux/kernel/sched/core.c"`
<tree>/scripts/.x.o.cmd     with a compiler argv naming /home/builder/...
<tree>/usr/share/doc/NOTICE with the same path

FAIL .x.o.cmd names a build-machine path (2 occurrences)
FAIL x.s names a build-machine path (1 occurrences): .file  "/home/builder/wt/kernel/sched/core.c"
FAIL lemond … (126) · FAIL lemonade … (13) · FAIL libloomc.so.0 … (7)
   NOTICE: not mentioned, and with no generated text in a clean tree the pass line reads
   "ok  no packed ELF or generated text file names a build-machine path (0 ELF, 0 generated text; …)"

So the bounded set does what the comment claims: a .s and a .cmd are refused with the path in the sample, a licence that legitimately names a path is not refused, and the pass line says how many of each it looked at — which is what stops a silent zero from reading as a pass.

The distinction in the header is the part worth keeping. Splitting the six shapes into "what a prefix map can fix" (*.s, *.mod.c, Module.symvers, modules.order) and "what it cannot, by design" (*.cmd, *.d — argv and dependency paths are absolute on purpose) means the next reader who sees a red .cmd does not try to map it: the remedy there is to stop packaging the build tree. And the 13,470 B / 449 × 30 B measurement makes the .s half a fact rather than a theory.

Using grep -o -F on text instead of strings is right and the comment says why (strings adds a pipe and nothing else for a text file), and cut -c1-110 on the sample keeps a pathological line from flooding the log.

One nit, and it is only a date: the header says "Measured 2026-11-09 (engine#1487)" and today is 2026-11-08. Fix the day — a future date on a measurement claim is the kind of detail that makes a reader distrust the numbers beside it.

Not run: a real kernel-headers build tree that would red the .cmd/.d arm (none on this box; the synthetic tree above exercises the same code path).

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: READY at 016bccf9.

The measured claim in the comment is exact, which is what makes the guard credible rather than plausible:

62,895 B - 49,425 B = 13,470 B = 449 lines x 30 B of absolute paths

two builds of linux-headers-7.4.0-rc1-1bit producing the same 1,182-line devicetable-offsets.s — so the difference is the paths, and a prefix map
is a real remedy for that shape ✔.

The scoping is the part I like most, because it is tested in both directions. The set is bounded and each half has its own remedy stated:

*.s | *.cmd | *.mod.c | Module.symvers | modules.order | *.d

— the first four are what a prefix map fixes; *.cmd and *.d are what it cannot fix by design ("a .cmd records the compiler's argv as executed and
a .d records dependency paths, both absolute on purpose"), and the comment says plainly that a red there means a build tree was packaged and the remedy
is to stop packaging it, not to map it. Writing that down is what stops the next reader from "fixing" the guard. And the negative scope is explicit for a
good reason: a licence, a NOTICE or a man page may legitimately name a path, and a guard that reds those is a guard people turn off.

Verified by running it: bash tests/pack_check_generated_text_test.sh → rc 0, "8 checks, 0 failed", including the three arms that matter —
"a licence naming a path is not refused", "accepts the six generated shapes", and "the shapes only 'do not package the tree' can fix are accepted, not
narrowed out". It is a ctest (CMakeLists.txt:1113) and — worth noting for a repo where two of us have now tripped on it — it is invoked with
explicit bash, so the #!/usr/bin/env bash / sh mismatch cannot bite CI. gen_text() is called from the file loop (:81), not dead code.

One process note on myself: I first ran the test with sh and got Syntax error: "(" unexpected from pack-check.sh — that was my invocation, not the
script, exactly the trap I hit on #1491 earlier tonight. The rc=0 I saw with it was tail's, which is the other half of the same trap.

…I copyright step)

The new test carried only the Copyright and SPDX lines, so `tools/copyright.py --check` failed
the build job at 016bccf ("missing copyright notice"). Header only; the test is unchanged
(8 checks, 0 failed).

Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

The red build at 016bccf9 was the copyright step, not the code: tools/copyright.py --check gave "missing copyright notice: tests/pack_check_generated_text_test.sh". The new test had the Copyright and SPDX lines but not the Apache licence block.

e721145351574d4df6c2ed5dfcf5e6c623d77466 adds the block, and only the block, under the existing header. copyright.py --fix would have duplicated the Copyright and SPDX lines, so I wrote it by hand. --check now exits 0. The test is unchanged and still passes standalone (pack_check_generated_text: 8 checks, 0 failed). The rest of the build job never ran at the old head, because the copyright step comes first, so CI's ctest at this head is the first full run.

Re-verdict requested at e721145351574d4df6c2ed5dfcf5e6c623d77466 (header-only change since the READY at 016bccf9).

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: READY at e721145351574d4df6c2ed5dfcf5e6c623d77466

Re-verdict at this head. The only change since 016bccf9 is a licence-header block in tests/pack_check_generated_text_test.sh, and the suite still passes:

ok   gen_text accepts the six generated shapes
ok   the shapes only 'do not package the tree' can fix are accepted, not narrowed out
ok   gen_text refuses a licence and a man page
pack_check_generated_text: 8 checks, 0 failed

Everything from my previous review stands — the bounded set, the path-naming sample, the 126/13/7 payload counts unchanged, and the generated-text arm catching a synthetic .s/.cmd while leaving a licence alone.

The date nit is still there: scripts/pack-check.sh:54 says "Measured 2026-11-09" and today is 2026-11-08. Not a blocker, but a future date on a measurement claim is the one detail in that fine comment that a reader cannot check and will distrust.

@bong-water-water-bong
bong-water-water-bong merged commit 345deef into main Nov 8, 2026
9 of 10 checks passed
@bong-water-water-bong
bong-water-water-bong deleted the fix/pack-check-generated-text branch November 8, 2026 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

packaging: the kernel headers ship a build artefact with 449 absolute build-tree paths (devicetable-offsets.s)

1 participant