RE: [PATCH 4/6] Under conditions of high load and low resources, it was possible for NDIS (in combination with overlying drivers) to send NET_BUFFER_LIST structures containing NULL MDL's for transmission. This resulted in an immediate bugcheck.

["Durrant, Paul" <pdurrant@xxxxxxxxxxxx>]

> -----Original Message-----
> From: win-pv-devel <win-pv-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf Of 
> Martin Harvey
> Sent: 23 July 2021 12:00
> To: paul@xxxxxxx; win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Owen Smith <owen.smith@xxxxxxxxxx>
> Subject: RE: [EXTERNAL] [PATCH 4/6] Under conditions of high load and low 
> resources, it was possible
> for NDIS (in combination with overlying drivers) to send NET_BUFFER_LIST 
> structures containing NULL
> MDL's for transmission. This resulted in an immediate bugcheck.
> 
> CAUTION: This email originated from outside of the organization. Do not click 
> links or open
> attachments unless you can confirm the sender and know the content is safe.
> 
> 
> 
> Hi Paul,
> 
> Okay, so in that case, you want to move the fix up the stack from xenvif to 
> xennet, in which case, my
> notes tell me that the call stack is as follows:
> 
> TransmitterQueuePacket - MDL parameter is NULL.
> VifTransmitterQueuePacket (MDL argument).
> 
> Called via VIF_INTERFACE:
> xennet!__TransmitterSendNetBufferList
> XENVIF_VIF(TransmitterQueuePacket( .... , NET_BUFFER_CURRENT_MDL(NetBuffer)
> 
> So, you think that the fix would best be placed in xennet! 
> __TransmitterSendNetBufferList, with an
> extra check that NET_BUFFER_CURRENT_MDL(NetBuffer) is not NULL.

Yes, exactly. NDIS weirdness like this should be dealt with at that level.

  Paul

> 
> MH.
> 
> -----Original Message-----
> From: win-pv-devel <win-pv-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf Of 
> Paul Durrant
> Sent: 21 July 2021 18:20
> To: win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH 4/6] Under conditions of high load and low resources, it 
> was possible for NDIS (in
> combination with overlying drivers) to send NET_BUFFER_LIST structures 
> containing NULL MDL's for
> transmission. This resulted in an immediate bugcheck.
> 
> [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments 
> unless you have verified the
> sender and know the content is safe.
> 
> Coping with NDIS is XENNET's responsibility. Any remedial action should be 
> taken there, not here.
> 
>    Paul
> 
> On 20/07/2021 14:29, Martin Harvey wrote:
> > This patch contains the immediate proximate fix for this particular
> > issue, instead failing the send with STATUS_INVALID_PARAMETER.
> >
> > Signed-off-by: Martin Harvey <martin.harvey@xxxxxxxxxx>
> > ---
> >   src/xenvif/transmitter.c | 9 ++++++++-
> >   1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/xenvif/transmitter.c b/src/xenvif/transmitter.c index
> > 724615d..ea1282c 100644
> > --- a/src/xenvif/transmitter.c
> > +++ b/src/xenvif/transmitter.c
> > @@ -5148,13 +5148,17 @@ TransmitterQueuePacket(
> >       PXENVIF_TRANSMITTER_RING        Ring;
> >       NTSTATUS                        status;
> >
> > +    status = STATUS_INVALID_PARAMETER;
> > +    if (Mdl == NULL)
> > +        goto fail1;
> > +
> >       Frontend = Transmitter->Frontend;
> >
> >       Packet = __TransmitterGetPacket(Transmitter);
> >
> >       status = STATUS_NO_MEMORY;
> >       if (Packet == NULL)
> > -        goto fail1;
> > +        goto fail2;
> >
> >       Packet->Mdl = Mdl;
> >       Packet->Offset = Offset;
> > @@ -5206,6 +5210,9 @@ TransmitterQueuePacket(
> >
> >       return STATUS_SUCCESS;
> >
> > +fail2:
> > +    Error("fail2\n");
> > +
> >   fail1:
> >       Error("fail1 (%08x)\n", status);
> >
> >
>