Re: [PATCH 1/3] Capture Context->Request once per SyncWorker iteration

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

On 16/09/2026 00:48, Tu Dinh wrote:
> Context->Request is a shared variable and so we must capture it instead
> of rereading every time. Otherwise, this sequence will be possible:
> * A synchronized action (e.g. __SyncProcessorDisableInterrupts)
>    completes from the owner's viewpoint, but its worker has not finished
>    updating Request = Context->Request yet;
> * The owner sets Context->Request to something else;
> * Request = Context->Request executes and sets Request to the wrong
>    value.
> 
> Assisted-by: LLM
> Signed-off-by: Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx>
> ---
>   src/xenbus/sync.c | 12 +++++++-----
>   1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/src/xenbus/sync.c b/src/xenbus/sync.c
> index eefa2fe..e034153 100644
> --- a/src/xenbus/sync.c
> +++ b/src/xenbus/sync.c
> @@ -284,21 +284,23 @@ SyncWorker(
>   
>       Request = SYNC_REQUEST_NONE;
>       for (;;) {
> -        NTSTATUS status;
> +        SYNC_REQUEST    Next;
> +        NTSTATUS        status;
>   
>           KeMemoryBarrier();
> +        Next = Context->Request;

This read should be replaced with ReadAcquire((LONG *)&Context->Request) 
in order to ensure that the fetch itself is done exactly once as opposed 
to being invented by the compiler. ReadAcquire also makes the full 
barrier unnecessary. I'll send a v2.

>   
> -        if (Context->Request == SYNC_REQUEST_EXIT)
> +        if (Next == SYNC_REQUEST_EXIT)
>               break;
>   
> -        if (Context->Request == Request) {
> +        if (Next == Request) {
>               _mm_pause();
>               continue;
>           }
>   
>           status = STATUS_SUCCESS;
>   
> -        switch (Context->Request) {
> +        switch (Next) {
>           case SYNC_REQUEST_DISABLE_INTERRUPTS:
>               status = __SyncProcessorDisableInterrupts(&Irql);
>               break;
> @@ -321,7 +323,7 @@ SyncWorker(
>           }
>   
>           if (NT_SUCCESS(status))
> -            Request = Context->Request;
> +            Request = Next;
>       }
>   
>       ASSERT3U(KeGetCurrentIrql(), ==, DISPATCH_LEVEL);



--
Ngoc Tu Dinh | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech