Unsynchronized SMP race on xmitWin causes heap OOB write on timeSent[] in ng_pptpgre

Summary

The legacy ng_pptpgre node has no per-node serialization (no spinlock, token, or mutex) protecting its mutable state (xmitWin, timeSent[], sequence numbers). On an SMP system, two GRE packets can be processed concurrently on different CPUs, each invoking ng_pptpgre_recv on the same node. The xmitWin growth check at line 672 (a->xmitWin < PPTP_XMIT_WIN) followed by the non-atomic increment at line 674 is a classic TOCTOU: two concurrent ack handlers can both read xmitWin == 15, both pass the < 16 check, and both increment, yielding xmitWin == 17. The subsequent xmit path then permits timeSent index 16, writing 8 bytes past the end of the pptptime_t timeSent[PPTP_XMIT_WIN] array and corrupting the recvSeq/xmitSeq fields in the adjacent priv struct memory.

Root cause

The entire node operates without any concurrency protection.

• ng_send_data() (sys/netgraph/netgraph/ng_base.c:1678) calls the peer hook's rcvdata with no lock — it just checks HK_INVALID and dispatches directly.
• The ksocket upcall (sys/netgraph/ng_ksocket/ng_ksocket.c:982, ng_ksocket_incoming) uses crit_enter() which is per-CPU only on DragonFlyBSD and does not prevent concurrent execution on other CPUs.
• sowakeup() (sys/kern/uipc_socket2.c:604) calls so->so_upcall without holding any serialization.

Therefore, two GRE packets arriving near-simultaneously on different CPUs both reach ng_pptpgre_recv concurrently.

The vulnerable sequence:

1. Window growth (sys/netgraph/pptpgre/ng_pptpgre.c:672-676): c if (PPTP_SEQ_DIFF(ack, a->winAck) >= 0 && a->xmitWin < PPTP_XMIT_WIN) { // CHECK a->xmitWin++; // INCREMENT (non-atomic) a->winAck = ack + a->xmitWin; } This read-check-increment on a->xmitWin (u_int16_t at :146) is non-atomic. Two concurrent invocations both see xmitWin == 15, both pass 15 < 16, both increment to produce xmitWin == 17.

2. Window check (sys/netgraph/pptpgre/ng_pptpgre.c:490-491): c if ((u_int32_t)PPTP_SEQ_DIFF(priv->xmitSeq, priv->recvAck) >= a->xmitWin) With xmitWin == 17, this allows xmitSeq - recvAck up to 16.

3. OOB write (sys/netgraph/pptpgre/ng_pptpgre.c:514): c a->timeSent[priv->xmitSeq - priv->recvAck] = ng_pptpgre_time(node); Index 16 writes to timeSent[16], one past the end of pptptime_t timeSent[PPTP_XMIT_WIN] (PPTP_XMIT_WIN = 16, valid indices 0..15). This is an 8-byte (sizeof u_int64_t) heap overflow within the priv allocation, overwriting recvSeq/xmitSeq — the fields immediately following ackp in struct ng_pptpgre_private (lines 165-168).

The timeSent array is the last field of struct ng_pptpgre_ackp (:152), and ackp is followed by recvSeq/xmitSeq/recvAck/xmitAck in struct ng_pptpgre_private (:165-168). The overflow corrupts these sequence-number fields with a 64-bit timestamp value.

Threat model & preconditions

• Attacker position: remote peer who can send GRE packets to the PPTP VPN concentrator's WAN interface. The GRE CID check at :632 requires the correct 16-bit call ID — this can be obtained by establishing a legitimate PPTP session (TCP 1723 control channel) or brute-forced (only 65536 possibilities).
• Privileges gained or impact: 8-byte kernel heap overflow corrupting recvSeq/xmitSeq, causing all subsequent sequence checks to fail (PPTP_SEQ_DIFF with a large timestamp value is always negative for normal seq numbers) → reliable PPTP session DoS. The overflow stays within the single priv kmalloc allocation (~200 bytes remaining after the write point), so it does not directly corrupt adjacent slab objects, limiting code-execution potential. The corrupted xmitSeq could in theory interact with other logic to cause further corruption if the attacker carefully times subsequent packets.
• Required config or capabilities: SMP DragonFly system (default on multi-core) with ng_pptpgre and ng_ksocket loaded, an active PPTP session with xmitWin grown to PPTP_XMIT_WIN - 1 = 15 (occurs naturally after ~15 successful ack round-trips during PPP negotiation), and two GRE ack packets arriving within the same few-CPU-instruction window on different CPUs.
• Reachability: send a burst of two raw GRE ack packets with the correct CID and an ack value at or beyond the current winAck threshold, simultaneously from two threads or via two raw sockets so they land on different RX queues / CPUs.

Proof of concept

PoC source: findings/poc/DF-0596/race.c (sketch — Linux attacker side, sending raw GRE via two threads; full driver to be materialized by the per-PoC verifier).

Build & run

# attacker (Linux + raw sockets, on the PPTP WAN-facing network):
cc -O2 -lpthread -o race race.c
./race <server_wan_ip> <cid> <ack_value>

# target (DragonFlyBSD PPTP concentrator, SMP guest with >= 2 vCPUs):
#   load ng_socket, ng_ksocket, ng_pptpgre, ng_iface, ng_ether (or ng_eiface)
#   configure a PPTP node graph per the standard mpd/netgraph PPTP recipe

Expected output

After the race succeeds (xmitWin now 17), subsequent server xmit trips the OOB write at :514. The server's recvSeq is overwritten with a large timestamp. All subsequent received GRE data packets fail the sequence check at :691 and are silently dropped (recvOutOfOrder++). The PPTP session is dead. On a DEBUG kernel, the corrupted sequence numbers may trigger KASSERT failures.

# server-side dmesg (DEBUG kernel):
panic: ... KASSERT in ng_pptpgre_xmit / ng_pptpgre_recv
backtrace:
    ng_pptpgre_xmit+0x...
    ng_pptpgre_recv+0x...
    ng_send_data+0x...

Impact

• Blast radius: any SMP DragonFly system acting as a PPTP VPN concentrator/server using the legacy netgraph ng_pptpgre node (mpd, custom netgraph scripts). PPTP is deprecated in favor of IPsec/L2TP but remains in active use on legacy networks and embedded VPN boxes.
• Severity rationale: Medium. Remote attacker, high race complexity (the two ack handlers must execute the check-increment window :672-676 within the same ~10-instruction window on different CPUs — expect O(1000-10000) attempts on a 2-vCPU guest under moderate load), impact limited to PPTP session DoS via the corrupted sequence numbers. No demonstrated code-execution primitive (overflow stays within the priv allocation, corrupting only sequence-number fields). CVSS 3.1 base ≈ 6.6 (Medium).
• Reliability: speculative — race is probabilistic; concrete reproducibility to be established by the per-PoC verifier on a live DragonFly guest.

Recommended fix

Add a per-node spinlock to serialize access to the mutable ackp/sequence state. The lock must be acquired in ng_pptpgre_recv, ng_pptpgre_xmit, ng_pptpgre_send_ack_timeout, and ng_pptpgre_recv_ack_timeout. Since ng_pptpgre_xmit is called from ng_pptpgre_recv (:709) and from ng_pptpgre_send_ack_timeout (:933), use a recursive lock or restructure to avoid self-deadlock. Alternatively, use a spinlock with careful lock-ordering (release before calling NG_SEND_DATA which may re-enter the node).

Minimal targeted fix using a spinlock:

--- a/sys/netgraph/pptpgre/ng_pptpgre.c
+++ b/sys/netgraph/pptpgre/ng_pptpgre.c
@@ -159,6 +159,7 @@ typedef u_int64_t       pptptime_t;
 struct ng_pptpgre_private {
    hook_p          upper;      /* hook to upper layers */
    hook_p          lower;      /* hook to lower layers */
+   struct spinlock     lock;       /* protects ackp/seq/window state */
    struct ng_pptpgre_conf  conf;       /* configuration info */
    struct ng_pptpgre_ackp  ackp;       /* packet transmit ack state */
    u_int32_t       recvSeq;    /* last seq # we rcv'd */
@@ -285,6 +286,7 @@ ng_pptpgre_constructor(node_p *nodep)
    if (priv == NULL)
        return (ENOMEM);

+   spin_init(&priv->lock, "ng_pptpgre");
    /* Call generic node constructor */
    if ((error = ng_make_node_common(&ng_pptpgre_typestruct, nodep))) {
        kfree(priv, M_NETGRAPH);

Then in ng_pptpgre_recv, acquire priv->lock after the enabled check and before touching any ackp/seq state; release before NG_SEND_DATA calls to avoid reentrancy deadlock. In ng_pptpgre_xmit, acquire priv->lock around the window check and timeSent write (lines 490-518). In the timer callbacks, acquire priv->lock around the state modifications (:828-847 and :932-933). Since the timer callbacks also call ng_pptpgre_xmit which would try to acquire the same lock, either make the spinlock recursive or extract the xmit logic into a lock-free helper (ng_pptpgre_xmit_locked) called with the lock already held.

A simpler but less complete mitigation is to make the xmitWin read-check-increment atomic:

-       if (PPTP_SEQ_DIFF(ack, a->winAck) >= 0
-           && a->xmitWin < PPTP_XMIT_WIN) {
-           a->xmitWin++;
-           a->winAck = ack + a->xmitWin;
+       if (PPTP_SEQ_DIFF(ack, a->winAck) >= 0) {
+           int old, new;
+           do {
+               old = a->xmitWin;
+               new = (old < PPTP_XMIT_WIN) ? old + 1 : old;
+           } while (atomic_cmpset_int(&a->xmitWin, old, new) == 0);
+           if (new != old)
+               a->winAck = ack + new;
        }

Note: this atomic fix only addresses the xmitWin bound; the other shared state (recvAck, xmitSeq, timeSent shift, rtt/dev/ato) still has data races that could cause logic corruption. The full spinlock fix is recommended.

References

• sys/netgraph/netgraph/ng_base.c:1678 (ng_send_data) — dispatches rcvdata inline on the caller's CPU with no serialization.
• RFC 2637 §3.2.7 (PPTP GRE windowing) and §4.4 (RTT/RTO estimator) — context for the window-grow sequence the race exploits.
• Same class as DF-0590 (legacy ng_bridge no-serialization races): the entire legacy netgraph tree lacks per-node locking for SMP.

Timeline

• 2026-07-02 Discovered during automated DragonFlyBSD kernel security audit.
• 2026-07-02 Reported to DragonFlyBSD security contact (pending).