[PATCH v2 1/1] ntdll: Fix reading stale Information from IOSB.

[Zebediah Figura]

On 5/30/22 14:20, Jinoh Kang wrote:
> +
> +    /* GetOverlappedResultEx() skips waiting for the event/file handle if the
> +     * Status field indicates completion, and returns the result from the
> +     * Information field.
> +     *
> +     * Hence, we have to ensure that writes to the Information field are seen
> +     * from the other threads *before* writes to the Status field.
> +     *
> +     * Otherwise, a race condition results: if writing the Status field has
> +     * been completed but the subsequent update to the Information field has
> +     * not, the application may end up reading stale value from it.
> +     *
> +     * The race condition can be especially problematic with applications that
> +     * use the HasOverlappedIoCompleted() macro in a tight loop to busy-wait
> +     * for completion of overlapped I/O.
> +     */
> +    MemoryBarrier();  /* TODO: consider replacing with a store-release. */
> +    *(volatile NTSTATUS *)status_ptr = status;
>   }
>   
>   static inline client_ptr_t iosb_client_ptr( IO_STATUS_BLOCK *io )

Unpaired MemoryBarrier() is pretty much always wrong. (In fact, I think 
unpaired anything would be, here.)

It also doesn't seem clear to me that it's actually better than a 
store-release, as you point out. Ideally what we want is ReadAcquire() 
and WriteRelease(), but even setting that aside, a pair of interlocked 
operations will probably be an improvement by themselves.