afd.sys AfdRestartGetAddress local address race

AFD, the kernel side of Winsock, caches each socket’s local transport address (the thing getsockname returns) so it doesn’t have to round-trip the transport on every query. On the per-socket AFD_ENDPOINT that cache is a pointer plus a length: LocalAddress (/* +0xf0 */) and LocalAddressLength (/* +0xec */). The bind path publishes the pair when a socket binds and clears it on unbind or on failed bind; the get-address completion routines AfdRestartGetAddress (the getsockname path) and AfdRestartBindGetAddress (the bind-completion path) read the pair to copy it back into the caller’s buffer. The bug is that the readers never agreed with the clearers on a lock for the pair.

The clearers — the failure-cleanup tail of AfdBind and the whole of AfdUnBindSocket — touch the pair under the global AfdGlobalData ERESOURCE only, and the readers take nothing at all. AfdBind’s success path, which publishes the pair, is itself lockless in both builds; that turns out to be tolerable because the publish stores the pointer before the length, so a reader that observes the new length is guaranteed to observe the new pointer. The clear path stores in the same order, but the danger is the reverse race, and that is what the readers walk into.

AfdUnBindSocket, vulnerable build. Note that the snapshot of the pointer to free is taken before any lock is acquired:

/* AfdUnBindSocket — vulnerable build */
PVOID poolToFree = ep->LocalAddress;             /* +0xf0  snapshot, NO lock */
ExEnterCriticalRegionAndAcquireResourceExclusive(AfdGlobalData);
ep->LocalAddress       = NULL;                   /* +0xf0 */
ep->LocalAddressLength = 0;                      /* +0xec */
ExReleaseResourceAndLeaveCriticalRegion(AfdGlobalData);
ExFreePoolWithTag(poolToFree, 'Afdl');

The AfdBind failure-cleanup tail is the same shape minus the snapshot — it just zeros the pair under the ERESOURCE. The reader holds nothing at all:

/* AfdRestartGetAddress — vulnerable build */
if (ep->EndpointType != 0xafd2 || ep->CachedAddrValid == 0)   /* +0x0, +0xc8 */
    return;

LONG  len  = ep->LocalAddressLength;                     /* +0xec  snapshot, NO lock */
ULONG copy = irp->MdlAddress->ByteCount - 4;             /* initial requested size */

if (copy <= (ULONG)len) {
    PVOID pa = ep->LocalAddress;                         /* +0xf0  load AFTER check */
    TdiCopyMdlToBuffer(irp->MdlAddress, 4, pa, 0, copy, &len);
}

copy = irp->IoStatus.Information - 4;                    /* +0x38  second pass */
if (copy <= (ULONG)len) {
    PVOID pa = ep->LocalAddress;                         /* reload, still no lock */
    TdiCopyMdlToBuffer(irp->MdlAddress, 4, pa, 0, copy, &len);
}

The ERESOURCE only serialises these clear paths against each other; it is irrelevant to an address query on another thread, because the reader never takes it. So between the length snapshot at the top and the pointer load inside the bounds-check branch, a concurrent unbind or bind-fail cleanup can zero both fields. The reader has already passed copy <= len against the old length; it then loads LocalAddress, which is now NULL, or a pointer to freed-and-reused pool, and hands it straight to TdiCopyMdlToBuffer.

The reliable outcome is the NULL deref — TdiCopyMdlToBuffer writing through NULL bugchecks, and winsock is open to any unprivileged user, so that is a clean local DoS. The more interesting outcome is the freed-and-reused variant: if the allocator hands the old buffer’s slot to another object before the copy runs, the copy reads/writes into that object — a UAF-shaped primitive whose elevation value depends on allocator timing, which I couldn’t turn into a reliable primitive here.

It is tempting to think the ERESOURCE should have made this safe. It can’t, for two reasons: the reader doesn’t take it, and even if it did, an ERESOURCE is not acquireable at the IRQL the completion routine runs at. The lock that fits both sides is the per-endpoint SpinLock (KeAcquireInStackQueuedSpinLock against &ep->SpinLock at +0x38), which neither side was using.

The patch

Both readers and both clear-writers now take the endpoint SpinLock around the pair. Reader (AfdRestartGetAddress; AfdRestartBindGetAddress is the same shape without the type/valid gate):

/* AfdRestartGetAddress — patched build */
KeAcquireInStackQueuedSpinLock(&ep->SpinLock, &Lock);     /* +0x38 */

PVOID pa = ep->LocalAddress;                              /* +0xf0 */
if (pa != NULL) {
    LONG  len  = ep->LocalAddressLength;                  /* +0xec */
    ULONG copy = irp->MdlAddress->ByteCount - 4;

    if (copy <= (ULONG)len)
        TdiCopyMdlToBuffer(irp->MdlAddress, 4, pa, 0, copy, &len);

    copy = irp->IoStatus.Information - 4;                 /* +0x38 */
    if (copy <= (ULONG)len)
        TdiCopyMdlToBuffer(irp->MdlAddress, 4, pa, 0, copy, &len);
}
KeReleaseInStackQueuedSpinLock(&Lock);

Clear-writer (AfdUnBindSocket; the AfdBind failure cleanup is the same shape minus the snapshot and free):

ExEnterCriticalRegionAndAcquireResourceExclusive(AfdGlobalData);
KeAcquireInStackQueuedSpinLock(&ep->SpinLock, &Lock);     /* +0x38 */
PVOID poolToFree = ep->LocalAddress;                      /* +0xf0  snapshot under lock */
ep->LocalAddress       = NULL;                            /* +0xf0 */
ep->LocalAddressLength = 0;                               /* +0xec */
KeReleaseInStackQueuedSpinLock(&Lock);
ExReleaseResourceAndLeaveCriticalRegion(AfdGlobalData);
ExFreePoolWithTag(poolToFree, 'Afdl');

Now the bounds decision, the pointer load and the copy are atomic against any bind/unbind: the reader sees either a valid (pointer, length) pair or NULL, never a length that belongs to a pointer that is already gone. The clear-writer’s own snapshot of the pointer (for the subsequent ExFreePoolWithTag) is moved inside the SpinLock too, so the clearer no longer frees a buffer another thread is mid-copy into. AfdBind’s success-path publish is left lockless — the patch accepts that as safe because the publish order (pointer, then length) cannot be torn in the dangerous direction.

Attack path

sequenceDiagram
    participant T1 as Thread A getsockname
    participant T2 as Thread B unbind or bind-fail cleanup
    participant K as AFD_ENDPOINT
    T1->>K: AfdRestartGetAddress reads LocalAddressLength = N, no lock
    T2->>K: AfdUnBindSocket zeros LocalAddress and LocalAddressLength under ERESOURCE
    T1->>K: bounds check against stale N passes, loads LocalAddress which is now NULL
    Note over K: TdiCopyMdlToBuffer writes through NULL, bugcheck

The shape to remember is narrower than “AFD needs locking”. It is: a pointer and a length that describe one buffer are coherent only if every accessor takes the same lock, and a bounds check separated from the dereference that depends on it is the exact place that breaks. Bind writes cached transport state, getsockname and the bind completion read it back asynchronously — every “cache the answer, hand it out later” structure in AFD is worth checking for the missing lock, and indeed the transmit-packet path in this same module got a matching fix this month (AfdStartNextTPacketsIrp) for the same reason.