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.

Thanks for the update.
Have a wonderful weekend
Oded

On Fri, Feb 27, 2026 at 2:03=E2=80=AFPM Simon Rozman <[email protected]=
i> wrote:
>
> 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=E2=80=AFAM 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 ringb=
uffer
> >>
> >>
> >> again thanks for providing the next release
> >> Regards,
> >> Oded
> >>
> >> On Thu, Feb 19, 2026 at 11:28=E2=80=AFPM Simon Rozman <simon.rozman@am=
ebis.si> 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() alon=
e
> >>> provide atomic manipulation with Ring->Head on all modern platforms.
> >>> Hence, these calls were made outside the spinlock, to squeeze an extr=
a
> >>> micrometer of performance.
> >>>
> >>> I could not directly apply your patch to the wintun repo, since it do=
es
> >>> 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 appl=
ied
> >>> it here:
> >>> https://git.zx2c4.com/wintun/commit/?id=3D607c181ea9fa0036d23598038e6=
019ad54db5ce4
> >>>
> >>> 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 mani=
pulate 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 =3D Ctx->Device.Send.Ring;
> >>>>        ULONG RingCapacity =3D Ctx->Device.Send.Capacity;
> >>>>
> >>>> +    KLOCK_QUEUE_HANDLE LockHandle;
> >>>> +    KeAcquireInStackQueuedSpinLock(&Ctx->Device.Send.Lock, &LockHan=
dle);
> >>>>        /* Allocate space for packets in the ring. */
> >>>>        ULONG RingHead =3D ReadULongAcquire(&Ring->Head);
> >>>> -    if (Status =3D NDIS_STATUS_ADAPTER_NOT_READY, RingHead >=3D Rin=
gCapacity)
> >>>> +    if (Status =3D NDIS_STATUS_ADAPTER_NOT_READY, RingHead >=3D Rin=
gCapacity) {
> >>>> +        KeReleaseInStackQueuedSpinLock(&LockHandle);
> >>>>            goto skipNbl;
> >>>> -
> >>>> -    KLOCK_QUEUE_HANDLE LockHandle;
> >>>> -    KeAcquireInStackQueuedSpinLock(&Ctx->Device.Send.Lock, &LockHan=
dle);
> >>>> +    }
> >>>>
> >>>>        ULONG RingTail =3D Ctx->Device.Send.RingTail;
> >>>>        ASSERT(RingTail < RingCapacity);
> >>>> @@ -419,8 +420,8 @@ TunReturnNetBufferLists(NDIS_HANDLE MiniportAdap=
terContext, PNET_BUFFER_LIST Net
> >>>>                Ctx->Device.Receive.ActiveNbls.Head =3D NET_BUFFER_LI=
ST_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(Comple=
tedNbl));
> >>>> +            KeReleaseInStackQueuedSpinLock(&LockHandle);
> >>>>                const MDL *TargetMdl =3D Ctx->Device.Receive.Mdl;
> >>>>                for (MDL *Mdl =3D NET_BUFFER_FIRST_MDL(NET_BUFFER_LIS=
T_FIRST_NB(CompletedNbl)); Mdl; Mdl =3D Mdl->Next)
> >>>>                {
> >>>
>