Re: [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.