Re: [win-pv-devel] [PATCH] Zero blkif ring before use

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: win-pv-devel [mailto:win-pv-devel-bounces@xxxxxxxxxxxxxxxxxxxx] On
> Behalf Of owen.smith@xxxxxxxxxx
> Sent: 29 November 2016 11:32
> To: win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Owen Smith <owen.smith@xxxxxxxxxx>
> Subject: [win-pv-devel] [PATCH] Zero blkif ring before use
> 
> From: Owen Smith <owen.smith@xxxxxxxxxx>
> 
> Also resets blkif ring's counters before freeing, as a safety measure
> for use-after-free attempting to access ring data.
> 
> Signed-off-by: Owen Smith <owen.smith@xxxxxxxxxx>

I'm going to nack this for the moment. There's no way a backend should be able 
to see the ring once it's torn down, because it will no longer be granted. OTOH 
the ring should be properly zeroed and initialized *before* granting in the 
first place. I need to check the code to see what level of safety we currently 
have.

  Paul

> ---
>  src/xenvbd/blockring.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/src/xenvbd/blockring.c b/src/xenvbd/blockring.c
> index 7cf5c84..416424d 100644
> --- a/src/xenvbd/blockring.c
> +++ b/src/xenvbd/blockring.c
> @@ -304,6 +304,7 @@ BlockRingConnect(
>      if (BlockRing->SharedRing == NULL)
>          goto fail2;
> 
> +    RtlZeroMemory(BlockRing->SharedRing, (SIZE_T)PAGE_SIZE <<
> BlockRing->Order);
>  #pragma warning(push)
>  #pragma warning(disable: 4305)
>  #pragma warning(disable: 4311)
> @@ -443,6 +444,11 @@ BlockRingDisconnect(
>      }
> 
>      RtlZeroMemory(&BlockRing->FrontRing, sizeof(BlockRing->FrontRing));
> +#pragma warning(push)
> +#pragma warning(disable: 4305)
> +#pragma warning(disable: 4311)
> +    SHARED_RING_INIT(BlockRing->SharedRing);
> +#pragma warning(pop)
>      __FreePages(BlockRing->SharedRing, BlockRing->Mdl);
>      BlockRing->SharedRing = NULL;
>      BlockRing->Mdl = NULL;
> --
> 2.8.3
> 
> 
> _______________________________________________
> win-pv-devel mailing list
> win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> https://lists.xenproject.org/cgi-bin/mailman/listinfo/win-pv-devel
_______________________________________________
win-pv-devel mailing list
win-pv-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/cgi-bin/mailman/listinfo/win-pv-devel