Bug fixes - #10
Merged
Merged
Conversation
The transport control_data pointer in WFP metadata is only valid for the duration of the classify callout. Previously we stored a raw NonNull<[u8]> into TransportPacketList, which was then used when injecting the packet after the user-space verdict — a use-after-free (BSOD 0xD1 in tcpip.sys). Fix: copy the control data into an owned Box<[u8]> via a new `get_control_data_copy()` helper on CalloutData, so the buffer remains valid through the deferred inject call.
Update heapless 0.8→0.9, num-derive 0.4→0.5, smoltcp 0.12→0.13 (adding medium-ethernet and socket-raw features), plus minor dep updates. Remove driver/Makefile.toml in favour of Justfile.
`next_id` and the lock-protected `values` vec were accessed in two separate steps. Acquiring the lock first ensures `next_id` is read while holding the lock, preventing a race where another thread could insert an entry and increment `next_id` between the read and the push.
Reauthorized outbound connections cannot be pended: WFP provides no completion handle, so the prior approach reset all filters on verdict, opening a write transaction that races into STATUS_FWP_TXN_IN_PROGRESS. Instead the ALE layer records the process id, emits an info-only event (no packet id) and permits. The packet layer handles the real packet and reinjects it after user space returns a verdict. Also rename build_loopback_info -> build_info_only since inbound loopback and reauth outbound share the same path, and extend the PacketDoc accordingly.
Inbound ALE callouts (accept_v4/v6) are unregistered. Inbound connections are now handled entirely by the packet layer: on a cache miss the packet layer creates a cache entry with process id 0 (unresolved; user space resolves it from the connection tuple), absorbs the packet and sends it to user space as a temporary verdict. The ALE functions are kept dead-code for easy re-enable. Update PacketDoc.md to reflect the new inbound flow.
Now that inbound ALE is disabled, remove the inbound-specific branches from `ale_layer_auth` and `save_packet`. Merge the Block/Drop verdict arms (both now permit into the packet layer), unify the reauthorize path (direction check no longer needed), and extract `add_connection` to deduplicate cache insertion. Mark the unused accept callouts with `#[allow(dead_code)]` and a note on what would need to be restored to re-enable them.
Replace the bare overlapped I/O in KextFile with a thread-safe runOverlapped helper that issues a per-operation manual-reset event, tracks in-flight requests with a WaitGroup, and cancels them before closing the handle. This eliminates a UAF window where the kernel could write to a freed buffer after the handle was closed. Fix waitForServiceStatus to poll correctly: the old loop exited immediately when the status already matched, and used a zero WaitHint (common for driver services) that turned polling into a busy-loop. The new loop queries first, then sleeps at least 100ms. Treat ERROR_SERVICE_NOT_ACTIVE on Stop as success so teardown does not report a spurious error when the driver was already stopped.
kext_interface version to 2.0.2
Deduplicate simultaneous inserts of the same connection that can race on the packet layer (multi-CPU): add_connection now checks for an existing equal entry under the write lock and, if found, merges the new observation (last-accessed time + bandwidth) instead of appending a duplicate. Supporting changes: - Add Connection::get_key() default method to build a Key from the accessor traits, eliminating per-type repetition. - Add BandwidthUsage::add_from() to accumulate counters when merging. - Extract add_connection, end_connection, end_all_on_port, set_connection_verdict, and find_verdict as shared generic helpers, replacing four copies of identical loop bodies in ConnectionCache. - Mark pend_filter_rest #[allow(dead_code)] and add a comment documenting its blocking side-effects. - Add a thread-safety note to KextFile::Read.
Switch `KextFile.mutex` from `sync.Mutex` to `sync.RWMutex` so that concurrent overlapped I/O operations can proceed in parallel. `runOverlapped` and `GetHandle` now hold a read lock; the write lock is reserved for the close transition. Extract `isClosed()` helper to reduce lock-free pre-check duplication.
Handle connection-creation failures in add_connection by logging an error instead of panicking, preventing a kernel panic on malformed ALE key data.
Previously the function read only the fixed 40-byte IPv6 header and assumed the next-header field was always an L4 protocol. This caused wrong protocol/port extraction for packets that carry Hop-by-Hop, Routing, Fragment, or Destination Options headers before the transport header. Walk the extension-header chain (up to MAX_IPV6_EXT_HEADERS deep, each at most MAX_IPV6_EXT_HEADER_LEN bytes) using read_prefix to build successive prefixes into a single stack-allocated buffer. Return an error for malformed or oversize chains.
Use the WFP ip_header_size metadata field instead of the fixed header constant when retreating the net buffer list to the IP header on inbound packets. IPv4 options and IPv6 extension header chains can push the transport header further forward, so retreating by only the minimum header length lands inside the header and causes every subsequent field parse to read shifted garbage. Changes: - `metadata.rs`: check `FWPS_METADATA_FIELD_IP_HEADER_SIZE` presence bit before reading the field; return `Option<u32>` so callers can distinguish "not provided" from zero. - `callout_data.rs`: propagate the `Option` return type. - `packet_callouts.rs`: use the metadata value when present and >= the fixed header length; fall back to the fixed constant only for absent or malformed values.
Split `ale_layer_auth` into `ale_layer_auth_outbound` and `ale_layer_auth_inbound`, and re-enable the previously commented-out AleAuthRecvAccept callouts (v4 + v6). Inbound TCP/UDP connections are now processed in the ALE layer instead of the packet layer: the packet layer permits unknown inbound packets so they reach the ALE receive-accept callout, which pends new connections and applies cached verdicts. Reauthorized inbound connections (no completion handle) fall back to `pend_filter_rest`; loopback connections cannot be pended at all and receive an immediate Accept with an info-only event sent to user space. `add_connection` gains an optional initial verdict so loopback entries can be inserted pre-accepted.
dhaavi
approved these changes
Aug 11, 2026
Comment on lines
+199
to
+218
| // A verdict exists: let the packet layer enforce it. For outbound connections the | ||
| // temporary verdicts (Accept, Block, Drop) and the redirects are all applied there. | ||
| Verdict::PermanentAccept | ||
| | Verdict::Accept | ||
| | Verdict::RedirectNameServer | ||
| | Verdict::RedirectTunnel | ||
| | Verdict::Block | ||
| | Verdict::Drop => { | ||
| data.action_permit(); | ||
| } | ||
| Verdict::PermanentBlock | Verdict::Undeterminable | Verdict::Failed => { | ||
| // Packet layer will not see this connection. | ||
| crate::dbg!("permanent block {}", key); | ||
| data.action_block(); | ||
| } | ||
| Verdict::PermanentDrop => { | ||
| // Packet layer will not see this connection. | ||
| crate::dbg!("permanent drop {}", key); | ||
| data.block_and_absorb(); | ||
| } |
There was a problem hiding this comment.
The comment does not really match the code here, right?
Temporary and redirects -> packet layer
Permanent or failure -> here
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
2 participants
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.
Summary
Collection of correctness and memory-safety fixes for the WFP kernel driver and its Go control interface, plus a restructuring of how inbound connections are classified.
Driver — Inbound Connections
AleLayerInboundV4/V6) are no longer registered. Inbound TCP/UDP connections are created and tracked entirely in the IP packet layer.0) and resolved by user space from the connection tuple.ale_layer_authis simplified to the outbound-only path:ale_layer_accept_v4/v6are retained (dead-coded) for reference.PacketDoc.mdupdated to match.Driver — Outbound Reauthorization
reset_all_filters) raced intoSTATUS_FWP_TXN_IN_PROGRESS.build_loopback_info→build_info_only(now also covers reauth).Driver — Control Data Use-After-Free Fix
classifycallout, but injection happens later.Box<[u8]>.TransportInjectContext: Owns the NBL, the control-data copy, and the remote address for the full lifetime of the async injection.free_transport_packetreclaims it in the completion routine.send_paramspointers are derived from the heap context rather than the stack.Driver — Connection Cache
add_connectionhelper: Does duplicate detection and publish under the port write lock, collapsing concurrent adds of the same connection into a single entry (fixes the duplicate-connection race).end_connection/end_all_on_port/set_connection_verdict/find_verdictto remove v4/v6 copy-paste.Connection::get_key()andBandwidthUsage::add_from().Driver — ID Cache Race Fix
IdCache::pushnow takes the write lock before reading/incrementingnext_id, closing a duplicate-ID data race.Go Interface — Overlapped I/O + Service Lifecycle
KextFilereworked for safe concurrent I/O and teardown:runOverlappedhelper.Cancel()andClose()that cancels outstanding I/O and waits for it to drain before closing the handle.deviceIOControlno longer returns the overlapped pointer.waitForServiceStatusrewritten:Stop()treatsERROR_SERVICE_NOT_ACTIVEas success.Build / Deps / Version
num-derive:0.4→0.5smoltcp:0.12→0.13(addsmedium-ethernet,socket-raw)driver/Makefile.toml.Justfilenow linksnexufend-agent.sys.kext_interfacebumped to2.0.2.