Re: [PATCH v2 1/2] Avoid ASSERT(IsZeroMemory()) in checked builds

[Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx>]

On 09/10/2026 12:42, Owen Smith wrote:
> * Zero Srb counters in BlkifRingDestroy
> * NULL Path pointer in BlkifRingCreate fail path
> * Zero embedded structures and values in FrontendDestroy
> * Zero TargetPath and FrontendPath on FrontendCreate fail path
> * Zero any Page80/83 overrides on FrontendConnect fail path
> * NULL TargetList entry in AdapterTeardown
> 
> Signed-off-by: Owen Smith <owen.smith@xxxxxxxxxx>

Both patches:

Reviewed-by: Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx>

> ---
>   src/xenvbd/adapter.c  |  1 +
>   src/xenvbd/frontend.c | 40 +++++++++++++++++++++++++++++-----------
>   src/xenvbd/ring.c     |  6 +++++-
>   3 files changed, 35 insertions(+), 12 deletions(-)
> 
> diff --git a/src/xenvbd/adapter.c b/src/xenvbd/adapter.c
> index 0f9500c..a0be841 100644
> --- a/src/xenvbd/adapter.c
> +++ b/src/xenvbd/adapter.c
> @@ -1507,6 +1507,7 @@ AdapterTeardown(
>           TargetSetMissing(Target, "AdapterTeardown");
>           // drop ref-count acquired in __AdapterGetTarget *before* 
> destroying Target
>           TargetDestroy(Target);
> +        Adapter->TargetList[TargetId] = NULL;
>       }
>   
>       Adapter->BootEmulated = FALSE;
> diff --git a/src/xenvbd/frontend.c b/src/xenvbd/frontend.c
> index 3b078da..5311dd5 100644
> --- a/src/xenvbd/frontend.c
> +++ b/src/xenvbd/frontend.c
> @@ -1287,6 +1287,19 @@ abort:
>   fail7:
>       Error("Fail7\n");
>       RtlZeroMemory(&Frontend->Features, sizeof(XENVBD_FEATURES));
> +
> +    if (Frontend->Page80.Data)
> +        Base64Free(Frontend->Page80.Data);
> +    Frontend->Page80.Data = NULL;
> +    Frontend->Page80.Size = 0;
> +
> +    if (Frontend->Page83.Data)
> +        Base64Free(Frontend->Page83.Data);
> +    Frontend->Page83.Data = NULL;
> +    Frontend->Page83.Size = 0;
> +
> +    RtlZeroMemory(&Frontend->DiskInfo, sizeof(XENVBD_DISKINFO));
> +
>   fail6:
>       Error("Fail6\n");
>   fail5:
> @@ -1923,8 +1936,10 @@ fail5:
>       Frontend->Ring = NULL;
>   fail4:
>       Error("fail4\n");
> +    RtlZeroMemory(Frontend->TargetPath, sizeof(Frontend->TargetPath));
>   fail3:
>       Error("fail3\n");
> +    RtlZeroMemory(Frontend->FrontendPath, sizeof(Frontend->FrontendPath));
>   fail2:
>       Error("Fail2\n");
>       Frontend->Target = NULL;
> @@ -1951,16 +1966,6 @@ FrontendDestroy(
>   
>       Trace("Target[%d] @ (%d) =====>\n", TargetId, KeGetCurrentIrql());
>   
> -    if (Frontend->Page80.Data)
> -        Base64Free(Frontend->Page80.Data);
> -    Frontend->Page80.Data = NULL;
> -    Frontend->Page80.Size = 0;
> -
> -    if (Frontend->Page83.Data)
> -        Base64Free(Frontend->Page83.Data);
> -    Frontend->Page83.Data = NULL;
> -    Frontend->Page83.Size = 0;
> -
>       ThreadAlert(Frontend->BackendThread);
>       ThreadJoin(Frontend->BackendThread);
>       Frontend->BackendThread = NULL;
> @@ -1971,11 +1976,24 @@ FrontendDestroy(
>       RingDestroy(Frontend->Ring);
>       Frontend->Ring = NULL;
>   
> -    Frontend->MaxQueues = 0;
> +    RtlZeroMemory(&Frontend->StateLock, sizeof(KSPIN_LOCK));
> +
> +    RtlZeroMemory(Frontend->TargetPath, sizeof(Frontend->TargetPath));
> +    RtlZeroMemory(Frontend->FrontendPath, sizeof(Frontend->FrontendPath));
> +    RtlZeroMemory(&Frontend->DiskInfo, sizeof(XENVBD_DISKINFO));
> +    RtlZeroMemory(&Frontend->Caps, sizeof(XENVBD_CAPS));
>   
>       ASSERT3P(Frontend->BackendPath, ==, NULL);
>       ASSERT3P(Frontend->BackendWatch, ==, NULL);
>   
> +    Frontend->Target = NULL;
> +    Frontend->TargetId = 0;
> +    Frontend->DeviceId = 0;
> +    Frontend->State = XENVBD_STATE_INVALID; // 0
> +    Frontend->BackendDomain = 0;
> +    Frontend->MaxQueues = 0;
> +
> +    ASSERT(IsZeroMemory(Frontend, sizeof(XENVBD_FRONTEND)));
>       __FrontendFree(Frontend);
>       Trace("Target[%d] @ (%d) <=====\n", TargetId, KeGetCurrentIrql());
>   }
> diff --git a/src/xenvbd/ring.c b/src/xenvbd/ring.c
> index 0b56831..ffbd18a 100644
> --- a/src/xenvbd/ring.c
> +++ b/src/xenvbd/ring.c
> @@ -1917,7 +1917,7 @@ fail4:
>       RtlZeroMemory(&(*BlkifRing)->SrbQueue, sizeof(LIST_ENTRY));
>   
>       __RingFree((*BlkifRing)->Path);
> -    (*BlkifRing)->Path;
> +    (*BlkifRing)->Path = NULL;
>       (*BlkifRing)->Index = 0;
>       (*BlkifRing)->Ring = NULL;
>   
> @@ -1957,6 +1957,10 @@ BlkifRingDestroy(
>       ASSERT(IsListEmpty(&BlkifRing->ShutdownQueue));
>       RtlZeroMemory(&BlkifRing->ShutdownQueue, sizeof(LIST_ENTRY));
>   
> +    BlkifRing->SrbsQueued = 0;
> +    BlkifRing->SrbsCompleted = 0;
> +    BlkifRing->SrbsFailed = 0;
> +
>       __RingFree(BlkifRing->Path);
>       BlkifRing->Path = NULL;
>       BlkifRing->Index = 0;



--
Ngoc Tu Dinh | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech