Re: [PATCH] Fix div by zero in xenvif if no queues.

["Durrant, Paul" <xadimgnik@xxxxxxxxx>]

On 15/03/2022 14:01, Martin Harvey wrote:
> In some cases (live migration), the front/backend can be pending
> reconnect at the same point as Tx tries to send packets.
There is locking that is supposed to stop that from happening. 
VifTransmitterQueuePacketVersion() calls AcquireMrswLockShared() to make 
sure that state is not changing underneath it.
> Previously, this caused a div by zero. However, since this is
> a transient error, the drivers should ditch the packet,
> and carry on.
It really should not happen at all.

  Paul

> Signed-off-by: Martin Harvey <martin.harvey@xxxxxxxxxx>
> ---
>   src/xenvif/frontend.c    | 19 ++++++++++++-------
>   src/xenvif/frontend.h    |  5 +++--
>   src/xenvif/transmitter.c |  8 +++++++-
>   3 files changed, 22 insertions(+), 10 deletions(-)
>
> diff --git a/src/xenvif/frontend.c b/src/xenvif/frontend.c
> index e38d2bf..fc4b78c 100644
> --- a/src/xenvif/frontend.c
> +++ b/src/xenvif/frontend.c
> @@ -2174,34 +2174,39 @@ fail1:
>       return status;
>   }
>   
> -ULONG
> +NTSTATUS
>   FrontendGetQueue(
>       IN  PXENVIF_FRONTEND                Frontend,
>       IN  XENVIF_PACKET_HASH_ALGORITHM    Algorithm,
> -    IN  ULONG                           Value
> +    IN  ULONG                           Value,
> +    OUT PULONG                          QueueIndex
>       )
>   {
> -    ULONG                               Queue;
> +    if (QueueIndex == NULL)
> +        return STATUS_INVALID_PARAMETER;
>   
>       switch (Algorithm) {
>       case XENVIF_PACKET_HASH_ALGORITHM_NONE:
>       case XENVIF_PACKET_HASH_ALGORITHM_UNSPECIFIED:
> -        Queue = Value % __FrontendGetNumQueues(Frontend);
> +        *QueueIndex = (__FrontendGetNumQueues(Frontend) != 0) ?
> +            Value % __FrontendGetNumQueues(Frontend) :
> +            0;
>           break;
>   
>       case XENVIF_PACKET_HASH_ALGORITHM_TOEPLITZ:
> -        Queue = (Frontend->Hash.Size != 0) ?
> +        *QueueIndex = (Frontend->Hash.Size != 0) ?
>                   Frontend->Hash.Mapping[Value % Frontend->Hash.Size] :
>                   0;
>           break;
>   
>       default:
>           ASSERT(FALSE);
> -        Queue = 0;
> +        *QueueIndex = 0;
>           break;
>       }
>   
> -    return Queue;
> +    return ((*QueueIndex) < __FrontendGetNumQueues(Frontend)) ?
> +        STATUS_SUCCESS : STATUS_INTERNAL_ERROR;
>   }
>   
>   static NTSTATUS
> diff --git a/src/xenvif/frontend.h b/src/xenvif/frontend.h
> index 8e5552e..5004b0e 100644
> --- a/src/xenvif/frontend.h
> +++ b/src/xenvif/frontend.h
> @@ -237,11 +237,12 @@ FrontendSetHashTypes(
>       IN  ULONG               Types
>       );
>   
> -extern ULONG
> +extern NTSTATUS
>   FrontendGetQueue(
>       IN  PXENVIF_FRONTEND                Frontend,
>       IN  XENVIF_PACKET_HASH_ALGORITHM    Algorithm,
> -    IN  ULONG                           Index
> +    IN  ULONG                           Value,
> +    OUT PULONG                          QueueIndex
>       );
>   
>   #endif  // _XENVIF_FRONTEND_H
> diff --git a/src/xenvif/transmitter.c b/src/xenvif/transmitter.c
> index ed89f60..d089301 100644
> --- a/src/xenvif/transmitter.c
> +++ b/src/xenvif/transmitter.c
> @@ -5194,13 +5194,19 @@ TransmitterQueuePacket(
>           break;
>       }
>   
> -    Index = FrontendGetQueue(Frontend, Algorithm, Value);
> +    status = FrontendGetQueue(Frontend, Algorithm, Value, &Index);
> +    if (!NT_SUCCESS(status))
> +        goto fail2;
> +
>       Ring = Transmitter->Ring[Index];
>   
>       __TransmitterRingQueuePacket(Ring, Packet, More);
>   
>       return STATUS_SUCCESS;
>   
> +fail2:
> +    Error("fail2\n");
> +
>   fail1:
>       Error("fail1 (%08x)\n", status);
>