18 Sep
2022
18 Sep
'22
3:11 p.m.
Ensure IOSB Information (InternalHigh) is loaded after Status (Internal)
to avoid reading stale value from the Information (InternalHigh) field.
--
v2: kernelbase: Fix reading stale information from IOSB.
https://gitlab.winehq.org/wine/wine/-/merge_requests/881
18 Sep
18 Sep
3:11 p.m.
New subject: [PATCH v2 1/1] kernelbase: Fix reading stale information from IOSB.
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com>
Ensure IOSB Information (InternalHigh) is loaded after Status (Internal)
to avoid reading stale value from the Information (InternalHigh) field.
---
dlls/kernelbase/file.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/dlls/kernelbase/file.c b/dlls/kernelbase/file.c
index 5ba7e0be419..11dbcdeac4a 100644
--- a/dlls/kernelbase/file.c
+++ b/dlls/kernelbase/file.c
@@ -3133,7 +3133,7 @@ BOOL WINAPI DECLSPEC_HOTPATCH GetOverlappedResultEx( HANDLE file,
OVERLAPPED *ov
TRACE( "(%p %p %p %lu %d)\n", file, overlapped, result, timeout, alertable
);
- status = overlapped->Internal;
+ status = ReadAcquire( (volatile LONG *)&overlapped->Internal );
if (status == STATUS_PENDING)
{
if (!timeout)
@@ -3150,7 +3150,13 @@ BOOL WINAPI DECLSPEC_HOTPATCH GetOverlappedResultEx( HANDLE file,
OVERLAPPED *ov
return FALSE;
}
- status = overlapped->Internal;
+ /* Normally this does not require an atomic load, since the wait above
+ * acts as a barrier as long as the caller supplied the correct handle
+ * and nothing else interfered with signalling of I/O completion.
+ * However, we do it anyway for consistency and extra safety in case
+ * Windows takes the same precaution for misbehaving applications.
+ */
+ status = ReadAcquire( (volatile LONG *)&overlapped->Internal );
if (status == STATUS_PENDING) status = STATUS_SUCCESS;
}
--
GitLab
https://gitlab.winehq.org/wine/wine/-/merge_requests/881
5:09 p.m.
New subject: [PATCH v2 1/1] kernelbase: Fix reading stale information from IOSB.
On 9/18/22 10:11, Jinoh Kang wrote:
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com>
Ensure IOSB Information (InternalHigh) is loaded after Status (Internal)
to avoid reading stale value from the Information (InternalHigh) field.
---
dlls/kernelbase/file.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
I don't think this should be a separate patch set as merge request 155,
and arguably not a separate commit from it either.
diff --git a/dlls/kernelbase/file.c
b/dlls/kernelbase/file.c
index 5ba7e0be419..11dbcdeac4a 100644
--- a/dlls/kernelbase/file.c
+++ b/dlls/kernelbase/file.c
@@ -3133,7 +3133,7 @@ BOOL WINAPI DECLSPEC_HOTPATCH GetOverlappedResultEx( HANDLE file,
OVERLAPPED *ov
TRACE( "(%p %p %p %lu %d)\n", file, overlapped, result, timeout,
alertable );
- status = overlapped->Internal;
+ status = ReadAcquire( (volatile LONG *)&overlapped->Internal );
if (status == STATUS_PENDING)
{
if (!timeout)
In general when using barriers I think it's good practice to comment (on
both sides) where the other half of the barrier is.
@@ -3150,7 +3150,13 @@ BOOL WINAPI DECLSPEC_HOTPATCH
GetOverlappedResultEx( HANDLE file, OVERLAPPED *ov
return FALSE;
}
- status = overlapped->Internal;
+ /* Normally this does not require an atomic load, since the wait above
+ * acts as a barrier as long as the caller supplied the correct handle
+ * and nothing else interfered with signalling of I/O completion.
+ * However, we do it anyway for consistency and extra safety in case
+ * Windows takes the same precaution for misbehaving applications.
+ */
+ status = ReadAcquire( (volatile LONG *)&overlapped->Internal );
if (status == STATUS_PENDING) status = STATUS_SUCCESS;
}
I don't see how this can even make a difference?
3 Oct
3 Oct
12:34 p.m.
In general when using barriers I think it's good
practice to comment (on both sides) where the other half of the barrier is.
ACK.
I don't see how this can even make a difference?
Right, misbehaving apps misbehave. My intention was to eliminate the possibility of a
specific execution order that possibly *cannot* be observed at all on Windows, so that
Wine's failure mode matches that of Windows. I gave up anyway.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/881#note_9952
12:34 p.m.
New subject: [PATCH v2 0/1] MR881: kernelbase: Fix reading stale information from IOSB. - closed
This merge request was closed by Jinoh Kang.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/881
608
days inactive
623
days old
4 comments
3 participants
participants (3)
-
Jinoh Kang -
Jinoh Kang (＠iamahuman)
-
Zebediah Figura