Re: [PATCH 1/1] in order to prevent buffer overrun (which was observed while sending multiple high throughput UDP streams from different threads) I move the driver spinlock to protect Ring buffer Head.

Yes, Oded. Your research is much appreciated. I have stare-reviewed it 
immediately, and it makes sense: by slightly extending the critical 
section a race condition should be fixed. Unfortunately, haven't had 
time to apply it to the master branch yet. Hopefully next week. 
Definitely before a release.

Lep pozdrav | Best regards,
Simon Rozman
Amebis, d. o. o., Kamnik

On 27.2.2026 22:28, Oded Katz wrote:
> Hi Simon,
> 
> We have found another issue with the driver code.
> some race condition between "alertable" flag between driver and
> user-side. causing driver to stall until new packet wakes it up.
> We have seen some cases where it can be for periods of 5 sec.
> 
> we already made the fix, and I sent 2 approaches for that fix.
> please let us know if you are planning to solve it in your repo?
> 
> regards and thanks,
> Oded
> 
> On Fri, Feb 20, 2026 at 8:55 AM Oded Katz <[email protected]> wrote:
>>
>> Hi Simon,
>>
>> Thanks for the update.
>> FYI: the ReadULongAcquire() and WriteULongRelease() themself are good
>> enough for getting and updating an atomic operation.
>> however, after on thread updates the head, the second thread can't
>> stay with the same atomic read value of the Head.
>> - so assuming 2 thread read the Head on the same time so the 2 threads
>> has same head.
>> - now 1st thread takes the spin lock and updates the head
>> - 2nd thread is pending the job until the spinlock is released and
>> then it doing the job, but with the non-updated Head value
>>       - so it will overrun the previous thread data and mess the ringbuffer
>>
>>
>> again thanks for providing the next release
>> Regards,
>> Oded
>>
>> On Thu, Feb 19, 2026 at 11:28 PM Simon Rozman <[email protected]> wrote:
>>>
>>> Hi, Oded!
>>>
>>> First and foremost, thank you very much for taking time to look into
>>> this and troubleshoot it.
>>>
>>> It was believed, that ReadULongAcquire() and WriteULongRelease() alone
>>> provide atomic manipulation with Ring->Head on all modern platforms.
>>> Hence, these calls were made outside the spinlock, to squeeze an extra
>>> micrometer of performance.
>>>
>>> I could not directly apply your patch to the wintun repo, since it does
>>> not follow our code style, commit message conventions and is not
>>> Signed-off-by you.
>>>
>>> However, it would have been a terrible waste of your research if your
>>> contribution wouldn't get into wintun, so I reworked your PR and applied
>>> it here:
>>> https://git.zx2c4.com/wintun/commit/?id=607c181ea9fa0036d23598038e6019ad54db5ce4
>>>
>>> Please, stay tuned for an official WHQL-signed release.
>>>
>>> Lep pozdrav | Best regards,
>>> Simon Rozman
>>> Amebis, d. o. o., Kamnik
>>>
>>> On 19. 2. 2026 20.32, odedkatz wrote:
>>>>       I observed that the Ring->Head was taken and manipulated later on with just a `ReadULongAcquire` which isn't OK when 2 are trying to manipulate it later on based on the same received value.
>>>> ---
>>>>    driver/wintun.c | 11 ++++++-----
>>>>    1 file changed, 6 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/driver/wintun.c b/driver/wintun.c
>>>> index d1f3b9f..65cd97e 100644
>>>> --- a/driver/wintun.c
>>>> +++ b/driver/wintun.c
>>>> @@ -284,13 +284,14 @@ TunSendNetBufferLists(
>>>>        TUN_RING *Ring = Ctx->Device.Send.Ring;
>>>>        ULONG RingCapacity = Ctx->Device.Send.Capacity;
>>>>
>>>> +    KLOCK_QUEUE_HANDLE LockHandle;
>>>> +    KeAcquireInStackQueuedSpinLock(&Ctx->Device.Send.Lock, &LockHandle);
>>>>        /* Allocate space for packets in the ring. */
>>>>        ULONG RingHead = ReadULongAcquire(&Ring->Head);
>>>> -    if (Status = NDIS_STATUS_ADAPTER_NOT_READY, RingHead >= RingCapacity)
>>>> +    if (Status = NDIS_STATUS_ADAPTER_NOT_READY, RingHead >= RingCapacity) {
>>>> +        KeReleaseInStackQueuedSpinLock(&LockHandle);
>>>>            goto skipNbl;
>>>> -
>>>> -    KLOCK_QUEUE_HANDLE LockHandle;
>>>> -    KeAcquireInStackQueuedSpinLock(&Ctx->Device.Send.Lock, &LockHandle);
>>>> +    }
>>>>
>>>>        ULONG RingTail = Ctx->Device.Send.RingTail;
>>>>        ASSERT(RingTail < RingCapacity);
>>>> @@ -419,8 +420,8 @@ TunReturnNetBufferLists(NDIS_HANDLE MiniportAdapterContext, PNET_BUFFER_LIST Net
>>>>                Ctx->Device.Receive.ActiveNbls.Head = NET_BUFFER_LIST_NEXT_NBL_EX(CompletedNbl);
>>>>                if (!Ctx->Device.Receive.ActiveNbls.Head)
>>>>                    KeSetEvent(&Ctx->Device.Receive.ActiveNbls.Empty, IO_NO_INCREMENT, FALSE);
>>>> -            KeReleaseInStackQueuedSpinLock(&LockHandle);
>>>>                WriteULongRelease(&Ring->Head, TunNblGetOffset(CompletedNbl));
>>>> +            KeReleaseInStackQueuedSpinLock(&LockHandle);
>>>>                const MDL *TargetMdl = Ctx->Device.Receive.Mdl;
>>>>                for (MDL *Mdl = NET_BUFFER_FIRST_MDL(NET_BUFFER_LIST_FIRST_NB(CompletedNbl)); Mdl; Mdl = Mdl->Next)
>>>>                {
>>>