[PATCH 0/1] MR4310: ntdll: Tweak the binary representation of SRWLOCK.

[Yuxuan Shui (＠yshui)]

There are applications that uses SRWLOCK in an invalid way and then checks its binary
representation. Specifically they releases an unlocked SRWLOCK then check its bit pattern is
all-ones.

Tweak the representation a bit so they are happy.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4310

[PATCH 1/1] ntdll: Tweak the binary representation of SRWLOCK.

[Yuxuan Shui]

From: Yuxuan Shui <yshui(a)codeweavers.com>

There are applications that uses SRWLOCK in an invalid way and then checks its binary
representation. Specifically they releases an unlocked SRWLOCK then check its bit pattern is
all-ones.

Tweak the representation a bit so they are happy.
---
 dlls/kernel32/tests/sync.c | 20 ++++++++++++++++++++
 dlls/ntdll/sync.c          | 27 ++++++++++++++-------------
 2 files changed, 34 insertions(+), 13 deletions(-)

diff --git a/dlls/kernel32/tests/sync.c b/dlls/kernel32/tests/sync.c
index 10765765bc5..6c0f2280f78 100644
--- a/dlls/kernel32/tests/sync.c
+++ b/dlls/kernel32/tests/sync.c
@@ -2535,6 +2535,25 @@ static void test_srwlock_example(void)
     trace("number of total exclusive accesses is %ld\n", srwlock_protected_value);
 }
 
+static void test_srwlock_quirk(void)
+{
+    union { SRWLOCK *s; LONG *l; } u = { &srwlock_example };
+
+    if (!pInitializeSRWLock) {
+        /* function is not yet in XP, only in newer Windows */
+        win_skip("no srw lock support.\n");
+        return;
+    }
+
+    *u.l = 0;
+    pReleaseSRWLockExclusive(&srwlock_example);
+    ok(*u.l == 0xffffffff, "expected 0xffffffff, got %lx\n", *u.l);
+
+    *u.l = 1;
+    pReleaseSRWLockExclusive(&srwlock_example);
+    ok(*u.l == 0, "expected 0x0, got %lx\n", *u.l);
+}
+
 static DWORD WINAPI alertable_wait_thread(void *param)
 {
     HANDLE *semaphores = param;
@@ -2887,6 +2906,7 @@ START_TEST(sync)
     test_condvars_base(&unaligned_cv.cv);
     test_condvars_consumer_producer();
     test_srwlock_base(&aligned_srwlock);
+    test_srwlock_quirk();
 #if defined(__i386__) || defined(__x86_64__)
     /* unaligned locks only work on x86 platforms */
     test_srwlock_base(&unaligned_srwlock.lock);
diff --git a/dlls/ntdll/sync.c b/dlls/ntdll/sync.c
index fa64917029a..eb05f185406 100644
--- a/dlls/ntdll/sync.c
+++ b/dlls/ntdll/sync.c
@@ -473,9 +473,7 @@ DWORD WINAPI RtlRunOnceExecuteOnce( RTL_RUN_ONCE *once, PRTL_RUN_ONCE_INIT_FN fu
 
 struct srw_lock
 {
-    short exclusive_waiters;
-
-    /* Number of shared owners, or -1 if owned exclusive.
+    /* Negated number of shared owners, or 1 if owned exclusive.
      *
      * Sadly Windows has no equivalent to FUTEX_WAIT_BITSET, so in order to wake
      * up *only* exclusive or *only* shared waiters (and thus avoid spurious
@@ -487,6 +485,8 @@ struct srw_lock
      * must not be the first element in the structure.
      */
     short owners;
+
+    short exclusive_waiters;
 };
 C_ASSERT( sizeof(struct srw_lock) == 4 );
 
@@ -532,7 +532,7 @@ void WINAPI RtlAcquireSRWLockExclusive( RTL_SRWLOCK *lock )
             if (!old.s.owners)
             {
                 /* Not locked exclusive or shared. We can try to grab it. */
-                new.s.owners = -1;
+                new.s.owners = 1;
                 --new.s.exclusive_waiters;
                 wait = FALSE;
             }
@@ -568,11 +568,11 @@ void WINAPI RtlAcquireSRWLockShared( RTL_SRWLOCK *lock )
             old.s = *u.s;
             new = old;
 
-            if (old.s.owners != -1 && !old.s.exclusive_waiters)
+            if (old.s.owners != 1 && !old.s.exclusive_waiters)
             {
                 /* Not locked exclusive, and no exclusive waiters.
                  * We can try to grab it. */
-                ++new.s.owners;
+                --new.s.owners;
                 wait = FALSE;
             }
             else
@@ -599,9 +599,10 @@ void WINAPI RtlReleaseSRWLockExclusive( RTL_SRWLOCK *lock )
         old.s = *u.s;
         new = old;
 
-        if (old.s.owners != -1) ERR("Lock %p is not owned exclusive!\n", lock);
+        if (old.s.owners != 1) ERR("Lock %p is not owned exclusive!\n", lock);
 
-        new.s.owners = 0;
+        /* This is how Windows unlocks and there are applications specifically depend on this. */
+        new.l--;
     } while (InterlockedCompareExchange( u.l, new.l, old.l ) != old.l);
 
     if (new.s.exclusive_waiters)
@@ -623,10 +624,10 @@ void WINAPI RtlReleaseSRWLockShared( RTL_SRWLOCK *lock )
         old.s = *u.s;
         new = old;
 
-        if (old.s.owners == -1) ERR("Lock %p is owned exclusive!\n", lock);
+        if (old.s.owners == 1) ERR("Lock %p is owned exclusive!\n", lock);
         else if (!old.s.owners) ERR("Lock %p is not owned shared!\n", lock);
 
-        --new.s.owners;
+        ++new.s.owners;
     } while (InterlockedCompareExchange( u.l, new.l, old.l ) != old.l);
 
     if (!new.s.owners)
@@ -654,7 +655,7 @@ BOOLEAN WINAPI RtlTryAcquireSRWLockExclusive( RTL_SRWLOCK *lock )
         if (!old.s.owners)
         {
             /* Not locked exclusive or shared. We can try to grab it. */
-            new.s.owners = -1;
+            new.s.owners = 1;
             ret = TRUE;
         }
         else
@@ -680,11 +681,11 @@ BOOLEAN WINAPI RtlTryAcquireSRWLockShared( RTL_SRWLOCK *lock )
         old.s = *u.s;
         new.s = old.s;
 
-        if (old.s.owners != -1 && !old.s.exclusive_waiters)
+        if (old.s.owners != 1 && !old.s.exclusive_waiters)
         {
             /* Not locked exclusive, and no exclusive waiters.
              * We can try to grab it. */
-            ++new.s.owners;
+            --new.s.owners;
             ret = TRUE;
         }
         else
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/4310