RE: [PATCH] Fix failure cleanup in EvtchnFifoAcquire

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

> -----Original Message-----
> From: win-pv-devel <win-pv-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf
> Of Tu Dinh
> Sent: Friday, July 18, 2025 2:08 AM
> To: win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx>; Owen Smith <owen.smith@xxxxxxxxx>
> Subject: [EXTERNAL] [PATCH] Fix failure cleanup in EvtchnFifoAcquire
> 
> 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.
> 
> 
> 
> The current index is incremented before checking for failure:
> 

I think that is by design.

>     while (Index < ProcessorCount) {
>         Index++;
>         [...]
>         if (!NT_SUCCESS(status))
>             goto fail1;
>         Context->ControlBlockMdl[vcpu_id] = Mdl;
>     }
> 
> Decrement the index before going into the cleanup loop to avoid calling
> __FreePage on invalid PMDLs.
> 
> Signed-off-by: Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx>
> ---
>  src/xenbus/evtchn_fifo.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/src/xenbus/evtchn_fifo.c b/src/xenbus/evtchn_fifo.c
> index 1137dea..ed78815 100644
> --- a/src/xenbus/evtchn_fifo.c
> +++ b/src/xenbus/evtchn_fifo.c
> @@ -561,6 +561,7 @@ fail1:
> 
>      EvtchnReset();
> 
> +    Index--;

I think this is wrong because...

>      while (--Index >= 0) {

... there is a pre-decrement here before the value of Index is used below.

Paul

>          unsigned int    vcpu_id;
> 
> --
> 2.50.1.windows.1
> 
> 
> 
> Ngoc Tu Dinh | Vates XCP-ng Developer
> 
> XCP-ng & Xen Orchestra - Vates solutions
> 
> web: https://vates.tech
>