Conversation
xdp_md.data is u32 in C, so ctx.data - k lowers to a 32-bit add on the loaded value, exactly as C ranks it, and the verifier rejects it: it tracks data as a packet pointer and prohibits 32-bit pointer arithmetic (observed in CI). IR and llc succeed, so the entry is at the verifier level. It flips to passing when packet-pointer fields get 64-bit pointer rank (TODO in expr_pass._descriptor). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
xdp_md.data/data_end/data_meta and __sk_buff.data/data_end/data_meta are u32 in C but packet pointers to the verifier, which rewrites their load into a pointer load and prohibits 32-bit arithmetic on them. The signedness rules ranked them as u32, so ctx.data - k lowered to a 32-bit add (w0 += -1) and was rejected; C code avoids this with a cast through (void *)(long). A binary operation with a packet-pointer operand now bypasses the usual arithmetic conversions: the pointer is used at 64 bits, an offset added to or subtracted from it is taken as an unsigned 16-bit value (truncated, zero-extended) because the verifier only tracks packet ranges up to MAX_PACKET_OFF (0xffff), and the result is a 64-bit value. Pointer minus pointer is an ordinary 64-bit length. A constant offset outside 0..65535, any other operator, and offset minus pointer are compile errors rather than code the verifier would reject. Local inference agrees (64-bit unsigned). Only direct field reads are recognised; a local copied from one is an integer again, and comparisons between packet pointers are the next site of the same rule. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The previous commit recognised a packet-pointer field only where it was read directly in a binary operation, so d = ctx.data made d an ordinary integer again and comparisons such as data + 34 > data_end were still ranked u32 and emitted as 32-bit compares, which the verifier rejects. PktPtrTy is a 64-bit unsigned IntTy tagged with the verifier's kind (pkt, pkt_meta, pkt_end). It originates at the field read and travels as the descriptor does: into a local's slot, through pointer +/- offset (which yields the same kind), and into comparisons, which compare at 64 bits unsigned whenever either side is a packet pointer. It is handled before the usual arithmetic conversions, which would re-rank it. Two rules the verifier enforces become compile errors: a packet-end pointer takes no offset, and convert() refuses to narrow a packet pointer below 64 bits. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…the refusals Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved validation gaps affect augmented assignment, unary negation, constant offsets, context classification, and pointer-kind propagation.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 3
Open (3)
What changed in this PR
Adds verifier-aware packet-pointer typing and arithmetic for XDP and socket-buffer context fields.
Changes:
- Introduces tagged packet-pointer types and lowering.
- Adds pointer arithmetic, bounds, comparison, and narrowing tests.
- Updates type inference, normalization, and local allocation.
| File | Summary |
|---|---|
tests/test_signedness_ir.py |
Adds packet-pointer IR assertions. |
tests/test_config.toml |
Registers expected failures. |
tests/passing_tests/vmlinux/ctx_data_via_local.py |
Tests pointer propagation through locals. |
tests/passing_tests/vmlinux/ctx_data_runtime_offset.py |
Tests runtime offsets. |
tests/passing_tests/vmlinux/ctx_data_plus_one.py |
Tests constant pointer arithmetic. |
tests/passing_tests/vmlinux/ctx_data_meta_offset.py |
Tests metadata-pointer offsets. |
tests/passing_tests/vmlinux/ctx_data_length.py |
Tests pointer subtraction. |
tests/passing_tests/vmlinux/ctx_data_arith.py |
Tests typed pointer arithmetic. |
tests/passing_tests/vmlinux/ctx_bounds_check.py |
Tests bounds checks through locals. |
tests/passing_tests/vmlinux/ctx_bounds_check_direct.py |
Tests direct bounds checks. |
tests/failing_tests/vmlinux/ctx_data_offset_too_big.py |
Tests oversized offset rejection. |
tests/failing_tests/vmlinux/ctx_data_narrow_store.py |
Tests narrowing rejection. |
tests/failing_tests/vmlinux/ctx_data_end_offset.py |
Tests packet-end arithmetic rejection. |
pythonbpf/type_deducer.py |
Defines packet-pointer types. |
pythonbpf/expr/type_normalization.py |
Validates packet-pointer conversions. |
pythonbpf/expr/type_inference.py |
Propagates pointer types. |
pythonbpf/expr/packet_pointer.py |
Maps context fields to pointer kinds. |
pythonbpf/expr/expr_pass.py |
Lowers pointer arithmetic and comparisons. |
pythonbpf/allocation_pass.py |
Allocates pointer-aware local storage. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+302
to
+305
| if isinstance(left_ty, PktPtrTy) or isinstance(right_ty, PktPtrTy): | ||
| # Before the usual arithmetic conversions, which would re-rank the | ||
| # pointer as an ordinary integer and drop its kind. | ||
| return _packet_pointer_binop(builder, rval, left, left_ty, right, right_ty) |
Comment on lines
+347
to
+350
| if isinstance(off, ir.Constant) and isinstance(off.constant, int): | ||
| if not 0 <= off.constant <= MAX_PACKET_OFF: | ||
| raise SyntaxError( | ||
| f"packet offset {off.constant} is outside 0..{MAX_PACKET_OFF} " |
Comment on lines
+50
to
+51
| every integer path accepts it; the few sites that must treat it as a | ||
| pointer check isinstance(ty, PktPtrTy) before the integer rules apply. |
Four keep-both conflicts: PktPtrTy and byte_size added at the same spot in type_deducer, the allocation pass's type_deducer import, and neighbouring entries in test_config.toml and test_signedness_ir.py. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
convert() refused to narrow a packet pointer below 64 bits, on the premise that the verifier only accepts one at 64 bits. That is true of arithmetic, not of stores: upstream's cgroup_skb_direct_packet_access stores skb->data_end into a __u32 global, and CI's verifier level accepted exactly that on #105. Merging master brought that selftest in and the refusal rejected it. Narrowing now truncates, and the result is an ordinary integer. ctx_data_narrow_store.py moves to the passing tests. Keeping the pointer when a local is declared narrower belongs to allocation (widest type ever stored), noted for later. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This branch has not been deployed
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.
Add a packet pointer type to IntTy