[PATCH] Fix possible infinite loop in hires_ms::timeGetTime_ns()

[PATCH] Fix possible infinite loop in hires_ms::timeGetTime_ns()

[Christian Franke]

[-- Attachment #1: Type: text/plain, Size: 1064 bytes --]

ntdll.h:SharedUserData misses a volatile qualifier. This (at least) may 
result in an infinite loop in hires_ms::timeGetTime_ns(). Fortunately 
this could only happen if LowPart wraps around during the function call.

Generated code:

$ objdump -d -C times.o
...
1160 <hires_ms::timeGetTime_ns()>:
1160: 55                 push   %ebp
1161: 8b 15 0c 00 fe 7f  mov    0x7ffe000c,%edx
1167: 3b 15 10 00 fe 7f  cmp    0x7ffe0010,%edx
116d: 89 e5              mov    %esp,%ebp
116f: a1 08 00 fe 7f     mov    0x7ffe0008,%eax
1174: 75 02              jne    1178 <hires_ms::timeGetTime_ns()+0x18>
1176: 5d                 pop    %ebp
1177: c3                 ret
1178: eb fe              jmp    1178 <hires_ms::timeGetTime_ns()+0x18>
...


This function results in the same code:

LONGLONG hires_ms::timeGetTime_ns ()
{
   LARGE_INTEGER t;
   t.HighPart = SharedUserData.InterruptTime.High1Time;
   t.LowPart = SharedUserData.InterruptTime.LowPart;
   if (t.HighPart == SharedUserData.InterruptTime.High2Time)
     return t.QuadPart;

   for (;;)
     ;
}


Christian


[-- Attachment #2: volatile-userdata.patch --]
[-- Type: text/x-patch, Size: 900 bytes --]

2012-03-20  Christian Franke  <franke@computer.org>

	* ntdll.h (SharedUserData): Add volatile qualifier. This fixes
	a possible infinite loop in hires_ms::timeGetTime_ns ().

diff --git a/winsup/cygwin/ntdll.h b/winsup/cygwin/ntdll.h
index d921867..7eee720 100644
--- a/winsup/cygwin/ntdll.h
+++ b/winsup/cygwin/ntdll.h
@@ -1106,9 +1106,10 @@ typedef VOID (APIENTRY *PTIMER_APC_ROUTINE)(PVOID, ULONG, ULONG);
 
 #ifdef __cplusplus
 /* This is the mapping of the KUSER_SHARED_DATA structure into the 32 bit
-   user address space.  We need it here to access the current DismountCount. */
-static KUSER_SHARED_DATA &SharedUserData
-			 = *(volatile PKUSER_SHARED_DATA) 0x7ffe0000;
+   user address space.  We need it here to access the current DismountCount
+   and InterruptTime.  */
+static volatile KUSER_SHARED_DATA &SharedUserData
+	= *(volatile KUSER_SHARED_DATA *) 0x7ffe0000;
 
 extern "C"
 {

Re: [PATCH] Fix possible infinite loop in hires_ms::timeGetTime_ns()

[Corinna Vinschen]

On Mar 20 18:48, Christian Franke wrote:
> ntdll.h:SharedUserData misses a volatile qualifier. This (at least)
> may result in an infinite loop in hires_ms::timeGetTime_ns().
> Fortunately this could only happen if LowPart wraps around during
> the function call.
> 
> Generated code:
> 
> $ objdump -d -C times.o
> ...
> 1160 <hires_ms::timeGetTime_ns()>:
> 1160: 55                 push   %ebp
> 1161: 8b 15 0c 00 fe 7f  mov    0x7ffe000c,%edx
> 1167: 3b 15 10 00 fe 7f  cmp    0x7ffe0010,%edx
> 116d: 89 e5              mov    %esp,%ebp
> 116f: a1 08 00 fe 7f     mov    0x7ffe0008,%eax
> 1174: 75 02              jne    1178 <hires_ms::timeGetTime_ns()+0x18>
> 1176: 5d                 pop    %ebp
> 1177: c3                 ret
> 1178: eb fe              jmp    1178 <hires_ms::timeGetTime_ns()+0x18>
> ...
> 
> 
> This function results in the same code:
> 
> LONGLONG hires_ms::timeGetTime_ns ()
> {
>   LARGE_INTEGER t;
>   t.HighPart = SharedUserData.InterruptTime.High1Time;
>   t.LowPart = SharedUserData.InterruptTime.LowPart;
>   if (t.HighPart == SharedUserData.InterruptTime.High2Time)
>     return t.QuadPart;
> 
>   for (;;)
>     ;
> }

Wow, thanks a lot for figuring this out and the patch.  This could
explain some spurious hangs.  Patch applied.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat