Skip to content

package: ship only the sealed NPU artifact surface (plaintext MLIR + patchelf abort) - #505

Merged
bong-water-water-bong merged 2 commits into
mainfrom
1bit/npu-package-allowlist
Oct 22, 2026
Merged

bong-water-water-bong merged 2 commits into
mainfrom
1bit/npu-package-allowlist

Conversation

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator

What

The release package carried the NPU kernel build tree, and the packaging run could not finish. Both come from one line: package-linux.sh did cp -a "$build/npu/kernels".

A sealed set directory still holds its layer1.work/ beside the sealed kernels. Measured on strixhalo 2026-10-22, per set: ~340 object files, ~250 intermediate AIE ELFs, 10 design.mlir.prj dirs, ~20 plaintext .mlir.

  1. The kernel method would have shipped in the clear. seal-kernels.sh only rewrites layer1_*/layer1.elf; everything in layer1.work/ stays plaintext, so the MLIR and object files went into a public artifact.
  2. Packaging aborted. pack_localize treats every staged ELF as dynamically linked and runs patchelf --set-rpath on it. An AIE object is static, so:
    patchelf: cannot find section '.dynamic'. The input file is most likely statically linked
    
    That is the 06:04 e2e run: engine-pkg/ empty, no PR. The weekly NPU-kernels PR stacked on npu: hardware-gate NPU phase + closed, encrypted kernel delivery #503 is blocked on this, not on "next".

Changes

  • scripts/package-linux.sh — stage the artifact surface explicitly: <set>/layout.json and <set>/layer1_*/layer1.elf, nothing else. (Loop variable is setname, not name: name is the package name tar uses further down.)
  • scripts/pack-elf.sh — new pack_is_dynelf; pack_localize only rewrites ELFs that actually have a .dynamic section, so a static ELF is skipped instead of aborting the package.
  • scripts/weekly.sh — package_checks asserts the exact allowlist under npu/kernels and names a leaked path; .mlir/.mlirbc now count as source files in the source scan.

Verified on strixhalo (main cfa197b1 scripts, existing e2e build tree)

check result
package-linux.sh rc=0, 1bit-e2e-linux-x86_64.tar.zst 16 MB
files under npu/kernels in the package 33 = 3 layout.json + 30 layer1.elf
sealed (1BPKCRYP magic) 30 / 30
.mlir, .o, layer1.work, design.mlir.prj 0
package_checks ok NPU kernels: 33 files under npu/kernels, all sealed
pack_is_dynelf yes on the real 1bit; no on an AIE object; no on a sealed kernel

Two other package_checks failures on that build are properties of the build, not of this change: its 1bit carries four /home/ build-machine paths baked in at compile time, and the tree has no onnx/ because it was configured without -DONEBIT_ONNX=ON.

Follow-up (separate repo, stacked on npu-kernels #15)

addons/release-kernels.sh still does cp -a "$cache/$name" "$out/$name", so a freshly built set is born dirty. Fixing it at the producer means the sealed tree on disk never holds plaintext MLIR either. This PR makes the consumer correct regardless, which is what unblocks the weekly run today.

The release package carried the NPU kernel build tree. package-linux.sh did
`cp -a "$build/npu/kernels"`, and a sealed set directory still holds its
layer1.work/ next to the sealed kernels: ~340 object files, ~250 intermediate
AIE ELFs, 10 design.mlir.prj dirs and ~20 plaintext .mlir per set (measured on
strixhalo 2026-10-22, all three sets). Two consequences:

1. The kernel method travels in the clear. Sealing only rewrites
   layer1_*/layer1.elf; everything in layer1.work/ stays plaintext, so the
   MLIR and objects would have shipped in a public artifact.
2. Packaging could not finish. pack_localize treats every staged ELF as
   dynamically linked and runs `patchelf --set-rpath` on it; an AIE object is
   static, so the run died with
   "patchelf: cannot find section '.dynamic'. The input file is most likely
   statically linked" - the 06:04 e2e run, engine-pkg/ empty, no PR.

Staging now copies the artifact surface explicitly: <set>/layout.json and
<set>/layer1_*/layer1.elf, nothing else. package_checks asserts that allowlist
on the tarball and names a leaked path, and .mlir/.mlirbc count as source files
in the source scan. pack_localize only rewrites ELFs that actually have a
.dynamic section (new pack_is_dynelf), so a static ELF is skipped instead of
aborting the package.

Verified on strixhalo against the e2e build (main cfa197b scripts, existing
build tree): package-linux.sh rc=0, 1bit-e2e-linux-x86_64.tar.zst 16 MB, and
the package now holds exactly 33 files under npu/kernels - 3 layout.json and 30
layer1.elf, every one sealed (1BPKCRYP magic), zero .mlir, .o, layer1.work or
design.mlir.prj. package_checks: "ok NPU kernels: 33 files under npu/kernels,
all sealed". pack_is_dynelf returns yes on the real 1bit, no on an AIE object
and no on a sealed kernel.

The two remaining package_checks failures on that build are properties of the
build, not of this change: its 1bit binary carries four /home/ build-machine
paths baked in at compile time, and the tree has no onnx/ because it was
configured without -DONEBIT_ONNX=ON.
@github-actions

github-actions Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

PR Reviewer Guide 🔍

(Review updated until commit 1773775)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

503 - Partially compliant

Compliant requirements:

  • The scripts/hw-gate.sh script implements the npu phase for hardware gating
  • The mechanism to verify sealed NPU kernels at runtime is implemented
  • NPU kernels are shipped as compiled, encrypted artifacts
  • Documentation updates are included in the PR
  • The sealed kernel delivery mechanism is verified on Strix Halo

Non-compliant requirements:

  • The weekly NPU-kernels PR is not yet integrated into scripts/weekly.sh to automatically include kernel build + seal + ship

Requires further human verification:

  • End-to-end run of the gate npu phase on the box
⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Potential packaging failure

The new pack_is_dynelf function checks for the presence of a .dynamic section to determine if an ELF file should be processed by patchelf. However, if a static ELF file is present in the package and the check fails, it will be silently skipped. This could lead to missing dependencies if the static ELF is required by other binaries in the package. Ensure that all necessary static dependencies are properly handled.

pack_is_dynelf() { pack_is_elf "$1" && readelf -S "$1" 2>/dev/null | grep -c '\.dynamic' | grep -qv '^0$'; }
Incorrect kernel artifact shipping

The script now explicitly stages only the artifact surface for NPU kernels, which is correct. However, the logic for copying the layout.json and layer1.elf files could be more robust. If a directory structure is malformed or a required file is missing, the script might silently skip the entire set, potentially leading to incomplete kernel delivery.

cp -a "$set/layout.json" "$stage/npu/kernels/$setname/"
for layer in "$set"/layer1_*/; do
    [ -f "$layer/layer1.elf" ] || continue
    mkdir -p "$stage/npu/kernels/$setname/$(basename "$layer")"
    cp -a "$layer/layer1.elf" "$stage/npu/kernels/$setname/$(basename "$layer")/"
Incomplete source file detection

The package_checks function now includes checks for .mlir and .mlirbc files in the tarball. However, the check for source files is not comprehensive enough. It only looks for files with specific extensions, but doesn't check for other potential source code artifacts or build artifacts that might be inadvertently included in the package.

-o -name '*.h' -o -name '*.hpp' -o -name '*.inc' -o -name '*.cu' -o -name '*.py' -o -name '*.cmake' \
-o -name '*.mlir' -o -name '*.mlirbc' \) | tee >(cat >&2) | wc -l)
Path resolution logic change

The built_path function has been changed to use relative paths instead of absolute paths. This change is intended to prevent build-machine paths from being embedded in the shipped binary. However, the new logic might not handle all edge cases, especially when the binary is run from a different directory than the build tree.

    const fs::path p = compiled;
#if defined(__linux__)
    if (!p.is_absolute()) {
        const fs::path here = fs::read_symlink("/proc/self/exe", ec).parent_path();
        if (!ec && fs::exists(here / p, ec)) return (here / p).string();
    }
#endif
    return std::string(compiled);   // an absolute (older build) or a path with no file beside us
Backend path compilation

The onebit_build_relative function is introduced to compile backend paths relative to the build directory. This is a good change to prevent build-machine paths from being embedded in the shipped binary. However, it's important to ensure that all backend paths are correctly handled by this function and that no paths are missed.

function(onebit_build_relative var out)
    file(RELATIVE_PATH _rel "${CMAKE_BINARY_DIR}" "${${var}}")
    set(${out} "${_rel}" PARENT_SCOPE)
endfunction()

…e paths

package_checks refuses a package whose 1bit names a build-machine home, and that
rule is what the NPU delivery tripped: of the four /home/ strings the e2e
package carried, two came from the NPU defines themselves
(ONEBIT_NPU_REPACK_SCRIPT / ONEBIT_NPU_LANE_PACK_SCRIPT = ${CMAKE_SOURCE_DIR}/...)
and the other two from ONEBIT_BUILD_DIR = ${CMAKE_BINARY_DIR}. So the package
gate failed on #503's own NPU path.

Every backend path is now embedded relative to the build dir via
file(RELATIVE_PATH ...) (a new onebit_build_relative() helper) and
app/built_path.h resolves it from the executable's own directory
(/proc/self/exe). That is the same file a 1bit in the build tree has always
found, because the binary lives in the build dir; the package keeps that layout
beside the binary (scripts/package-linux.sh, pack_localize). Nothing
machine-specific is compiled in any more.

Sites: ONEBIT_HRX_SERVER, ONEBIT_DS4_SERVER, ONEBIT_ONNX_SERVER,
ONEBIT_LAYA_GGML_BIN, the two NPU python tools. ONEBIT_BUILD_DIR is removed and
has no remaining references. Absolute paths still work if a caller passes one
(an older build), and the env overrides (ONEBIT_LLAMA_SERVER etc.) are
unchanged and still win.
@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit 1773775

@bong-water-water-bong
bong-water-water-bong merged commit f90d041 into main Oct 22, 2026
8 checks passed
@bong-water-water-bong
bong-water-water-bong deleted the 1bit/npu-package-allowlist branch October 22, 2026 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant