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.

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=E2=80=AFPM Simon Rozman <simon.rozman@amebis.=
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() 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=3D607c181ea9fa0036d23598038e6019a=
d54db5ce4
>
> 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 manipula=
te 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, &LockHandle=
);
> >       /* Allocate space for packets in the ring. */
> >       ULONG RingHead =3D ReadULongAcquire(&Ring->Head);
> > -    if (Status =3D NDIS_STATUS_ADAPTER_NOT_READY, RingHead >=3D RingCa=
pacity)
> > +    if (Status =3D NDIS_STATUS_ADAPTER_NOT_READY, RingHead >=3D RingCa=
pacity) {
> > +        KeReleaseInStackQueuedSpinLock(&LockHandle);
> >           goto skipNbl;
> > -
> > -    KLOCK_QUEUE_HANDLE LockHandle;
> > -    KeAcquireInStackQueuedSpinLock(&Ctx->Device.Send.Lock, &LockHandle=
);
> > +    }
> >
> >       ULONG RingTail =3D Ctx->Device.Send.RingTail;
> >       ASSERT(RingTail < RingCapacity);
> > @@ -419,8 +420,8 @@ TunReturnNetBufferLists(NDIS_HANDLE MiniportAdapter=
Context, PNET_BUFFER_LIST Net
> >               Ctx->Device.Receive.ActiveNbls.Head =3D NET_BUFFER_LIST_N=
EXT_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(CompletedN=
bl));
> > +            KeReleaseInStackQueuedSpinLock(&LockHandle);
> >               const MDL *TargetMdl =3D Ctx->Device.Receive.Mdl;
> >               for (MDL *Mdl =3D NET_BUFFER_FIRST_MDL(NET_BUFFER_LIST_FI=
RST_NB(CompletedNbl)); Mdl; Mdl =3D Mdl->Next)
> >               {
>