Repository navigation
Conversation
mfrw
marked this pull request as ready for review
November 10, 2026 10:18
kolyshkin
approved these changes
Nov 12, 2026
kolyshkin
left a comment
kolyshkin
left a comment
Contributor
There was a problem hiding this comment.
I actually like this one; tried improving it and returned to this version :)
LGTM, thanks!
The device filter only uses a handful of single-width eBPF instructions (word loads, 32-bit ALU operations, JNE and exit). Encode them directly instead of using cilium/ebpf/asm, and point each conditional jump at the next block by offset rather than by label. The generated bytecode does not change. cilium/ebpf/asm imports crypto/sha1 for Instructions.Tag, which is not used here. It is the only package in the runc binary that imports a crypto package. Remove endian.go as well: it was only needed because asm's Marshal does not accept binary.NativeEndian. The tests now decode the generated program with cilium/ebpf/asm, in both byte orders, and compare it with the existing expected listings, which are unchanged. cilium/ebpf is now only a test dependency. Signed-off-by: Muhammad Falak R Wani <falakreyaz@gmail.com>
kolyshkin
force-pushed
the
devices-no-asm
branch
from
November 12, 2026 23:14
2233706 to
d652a09
Compare
Author
Thank you kind sir |
There was a problem hiding this comment.
🟡 Changes recommended
A committed test is missing for restricted-access rules with both major and minor specified, leaving the longest jump-offset case unchecked in either byte order.
1 open issue
What changed in this PR
Replaces the cgroup device filter’s runtime use of cilium/ebpf/asm with direct instruction encoding. This preserves the generated bytecode while helping downstream runc avoid unused cryptographic dependencies.
Changes:
- Adds a compact eBPF instruction representation and byte-order-aware encoding.
- Replaces assembler labels with direct jump offsets and removes the native-endian workaround.
- Keeps the assembler as a test-only dependency to check existing listings in both byte orders.
| File | Description |
|---|---|
devices/endian.go |
Removes the assembler-specific native-endian workaround. |
devices/ebpf_linux.go |
Adds direct instruction encoding and updates program loading. |
devices/devicefilter.go |
Generates raw instructions with calculated jump offsets. |
devices/devicefilter_test.go |
Decodes both byte orders and compares existing expected listings. |
Review effort: Max
| t.Fatalf("%s: %v (devices: %+v)", t.Name(), err, devices) | ||
| } | ||
| s := insts.String() | ||
| s := disassemble(t, insts, binary.LittleEndian) |
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.
Draft for discussion in opencontainers/runc#5612.
The device filter only uses a few single-width eBPF instructions (word loads, 32-bit ALU operations, JNE and exit), so encode them directly instead of going through cilium/ebpf/asm. Each conditional jump points at the next block by offset instead of by label. The generated bytecode doesn't change.
cilium/ebpf/asm imports crypto/sha1 for
Instructions.Tag, which isn't used here, and it's the only package in the runc binary that imports a crypto package. opencontainers/runc#5612 explains why runc should link no crypto at all.endian.go goes away too; it was only there because asm's
Marshaldoesn't acceptbinary.NativeEndian.Testing:
go test ./...as root withCGROUPS_ALLOW_UNSAFE_TESTS=true, with and without-race; golangci-lint with.golangci.ymland.golangci-extra.yml.script/check-binary-size.shreports -477,688 bytes.