Bug fixes 2 - #11
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.
- Handle variable IPv4 header length (IHL field) when locating transport-layer ports in get_key_from_nbl_v4; previously always used the fixed 20-byte offset, misreading ports on packets with IP options - Validate IHL bounds before use to reject malformed headers - Move read_prefix helper before get_key_from_nbl_v4 so it is shared by both v4 and v6 paths (no behaviour change) - Make NetBufferList::retreat and NetworkAllocator::retreat_net_buffer return Result, propagating NdisRetreatNetBufferDataStart failures - In the inbound packet callout, treat a retreat failure as fatal: block-and-absorb the packet rather than continuing with a shifted data offset that would derive a key for the wrong connection
Previously, packet callouts operated on the head NET_BUFFER of each NET_BUFFER_LIST, silently skipping any subsequent buffers in the chain. This could cause missed packets and incorrect bandwidth accounting when a list carried more than one NET_BUFFER. Introduce a `NetBuffer` type wrapping `*mut NET_BUFFER` with its own `read_bytes`, `retreat`/`advance` (with auto-advance-on-drop), `get_data_length`, and `clone_as_nbl` methods. Add `NetBufferIter` to walk all net buffers in a list via `NetBufferList::iter_net_buffers`. Switch `packet_callouts.rs` to iterate `nbl.iter_net_buffers()` and operate on each `NetBuffer` individually. Rename `get_key_from_nbl_v4` / `get_key_from_nbl_v6` to `get_key_from_nb_v4` / `get_key_from_nb_v6` and update all call sites.
Separate filter removal from callout removal so pended operations can be completed in between. `FwpsCalloutUnregisterById0` fails while an operation is outstanding, and unloading over a live callout leaves the waiting thread stuck in kernel mode with no way to ever terminate it. Changes: - Add `shutdown_started` `AtomicBool` to `Device`; callouts that see it set permit instead of pending, so no new work lands in the packet cache while it is being drained. - Refactor `Device::shutdown` to: remove filters, rundown the event queue, drain the packet cache (10 × 10 ms passes to catch in-flight pends), then remove callouts. - Add `complete_pending_packets` helper extracted from the old `shutdown` body, now looping with a sleep to absorb operations that were already in flight when the filters were removed. - Call `shutdown` from `Device::Drop` so an unload that never received a shutdown command still tears down in the right order. - Add `FilterEngine::unregister_filters` and `FilterEngine::unregister_callouts`; the `Drop` failsafe now calls them in the correct order instead of interleaving the two steps. - Add `sleep_ms` via `KeDelayExecutionThread`; add `KeGetCurrentIrql`, `KERNEL_MODE`, and `APC_LEVEL` so IRQL is checked before waiting. - Null out the `DEVICE` pointer in `driver_unload` after the `Box` is freed.
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.
# Conflicts: # driver/src/packet_callouts.rs
Inbound reauthorized connections have no completion handle, so they are released by resetting all WFP filters. When several arrive together each one tries to open a write transaction; only one can be open at a time, so concurrent resets fail with STATUS_FWP_TXN_IN_PROGRESS and the connections are dropped. Fix by funneling every reset through a FilterResetQueue. One thread holds the drain claim and works the queue; it takes everything queued since the last reset in a single batch, so one reset releases all of them. Resets that still lose the transaction race are retried up to 20 times with a short sleep. ClearCache goes through the same queue since it issues the same reset. Also: distinguish InProgress from fatal transaction failures in the WFP layer; add begin_write_retrying for callers that must not lose the transaction race (commit, unregister); extract ntstatus_name helper; raise log level to Info.
dhaavi
approved these changes
Aug 11, 2026
In the outbound ALE auth callout, PermanentAccept was grouped with the temporary verdicts that get enforced at the packet layer. It is a permanent verdict and is applied at the ALE layer, so give it a separate arm with its own comment. Behaviour is unchanged — both arms permit. Also replace the product name in the rebrand-driver.ps1 doc comments with a generic placeholder, so the public repo never references it.
# Conflicts: # PacketDoc.md # driver/src/ale_callouts.rs # driver/src/packet_callouts.rs # driver/src/packet_util.rs # scripts/rebrand-driver.ps1 # wdk/src/filter_engine/callout_data.rs
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.
No description provided.