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.

Oded Katz <[email protected]> Fri, 27 Feb 2026 13:28:05 -0800
Newsgroups com.zx2c4.lists.wireguard
Message-ID <CAApnxP3H7LjxDBfFoPMsjZ=fb1mosOaemLW-c5xgdhuzWQvzfA@mail.gmail.com>
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]> wro=
te:
>
> 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 ringbuffe=
r
>
>
> again thanks for providing the next release
> Regards,
> Oded
>
> On Thu, Feb 19, 2026 at 11:28=E2=80=AFPM Simon Rozman <simon.rozman@amebi=
s.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 applie=
d
> > it here:
> > https://git.zx2c4.com/wintun/commit/?id=3D607c181ea9fa0036d23598038e601=
9ad54db5ce4
> >
> > 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 o=
n with just a `ReadULongAcquire` which isn't OK when 2 are trying to manipu=
late 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, &LockHand=
le);
> > >       /* Allocate space for packets in the ring. */
> > >       ULONG RingHead =3D ReadULongAcquire(&Ring->Head);
> > > -    if (Status =3D NDIS_STATUS_ADAPTER_NOT_READY, RingHead >=3D Ring=
Capacity)
> > > +    if (Status =3D NDIS_STATUS_ADAPTER_NOT_READY, RingHead >=3D Ring=
Capacity) {
> > > +        KeReleaseInStackQueuedSpinLock(&LockHandle);
> > >           goto skipNbl;
> > > -
> > > -    KLOCK_QUEUE_HANDLE LockHandle;
> > > -    KeAcquireInStackQueuedSpinLock(&Ctx->Device.Send.Lock, &LockHand=
le);
> > > +    }
> > >
> > >       ULONG RingTail =3D Ctx->Device.Send.RingTail;
> > >       ASSERT(RingTail < RingCapacity);
> > > @@ -419,8 +420,8 @@ TunReturnNetBufferLists(NDIS_HANDLE MiniportAdapt=
erContext, PNET_BUFFER_LIST Net
> > >               Ctx->Device.Receive.ActiveNbls.Head =3D NET_BUFFER_LIST=
_NEXT_NBL_EX(CompletedNbl);
> > >               if (!Ctx->Device.Receive.ActiveNbls.Head)
> > >                   KeSetEvent(&Ctx->Device.Receive.ActiveNbls.Empty, I=
O_NO_INCREMENT, FALSE);
> > > -            KeReleaseInStackQueuedSpinLock(&LockHandle);
> > >               WriteULongRelease(&Ring->Head, TunNblGetOffset(Complete=
dNbl));
> > > +            KeReleaseInStackQueuedSpinLock(&LockHandle);
> > >               const MDL *TargetMdl =3D Ctx->Device.Receive.Mdl;
> > >               for (MDL *Mdl =3D NET_BUFFER_FIRST_MDL(NET_BUFFER_LIST_=
FIRST_NB(CompletedNbl)); Mdl; Mdl =3D Mdl->Next)
> > >               {
> >