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.

[Martin Harvey <martin.harvey@xxxxxxxxxx>]

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.

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);
>   
>