Wednesday, March 5, 2014

Never, ever dismiss the most innocent failing test on your continuousintegration system!

Situation

As part of a project in C that requires some threading primitives, I wrote a piece of code to abstract a Windows timer into a small C class (yes, C class... please don't bash me here for mixing "classes" with plain C code, that will be material for another discussion).

You can find the header file of the basic classes provided by this library here:

https://gist.github.com/jsbattig/9379060

This timer primitive uses Windows threadpool timers. They are kind of nice because they have nothing to do with traditional windows handle linked timers which require a message loop marshaling messages sent to the handle neither they are "multimedia timers". Besides, this library already required thread pools so adding support for threadpool timers was a natural extension to the library itself.

By looking at Windows API documentation I find that to get one of these timers going, besides the fact you need a callback environment variable pre-initialized (or NULL) you need to use the following two APIs:

CreateThreadpoolTimer
SetThreadpoolTimer

SetThreadpoolTimer will actually "start" the timer.

The key parameters this function receives are the timer handle, a "due time" and a recurrence parameter or interval. There's a fourth parameter named msWindowLength which we don't particularly care at this point.

The due time, has the oddity to be the number of units of 100 nanoseconds since January 1, 1601 (UTC) expressed in FILETIME format (I personally found this kind of odd, for a timer... but anyway, MS folks did it like this for some reason I imagine). This parameter, if negative, can also mean a relative time since current time.
The msPeriod parameter is expressed in milliseconds. Why one parameter denoting timer timing in FILETIME and the other in milliseconds? Don't know...

Anyway, at first sight it seemed pretty straightforward, so I crafted the code bellow to get my timer created and going:


So, as you can quickly see, I missed the boat from the get going since I set the FILETIME parameter to be the actual interval in milliseconds converted to 100 nanosecond units since 0 (zero) time.

Here's where Mr Jenkins came to the rescue!
There's a particular test I run to exercise the functioning of this timer:

This test failed once today just before I wrote this blog entry.
This is the GoogleTest jenkins entry for the failure:

c_driver_SSvcBusThreadPoolTest_Win32_Release_singleMongo.SvcBusConsumer_testThreadPoolTimer

 Error Details

Value of: SvcBusThreadPoolTimer_getThreadId( timer)
  Actual: 24652
Expected: 0

 Stack Trace

threadpool_unittest.cc:132
Value of: SvcBusThreadPoolTimer_getThreadId( timer)
  Actual: 24652
Expected: 0

I could have easily dismiss it as a "fluke" or some kind of oddity, maybe the CPU was too busy and the program took to long to go from the creation of the timer to the actual check that's why the threadId recorded was != 0 (even tough the timer was created with 100ms due time).

I don't believe in "oddities" anymore at this point in my career and haven't done so for a while...

So, I dug a little bit to find the obvious which I pretty much imagined since I saw the Jenkins log.

The timer was kicking right away because I pass a positive number not adjusted to January 1st 1601. Now, I did write code to do the adjustment and showed the new working code to a colleague only for him to quickly point me down after checking the documentation why I didn't use the "relative to current" feature. Always good to have another pair of eyes checking at your code to uncover your own naiveness!

So, finally I did that, which by the way it was a really easy solution. All it took was casting the millis parameter to __int64 and negate it. That's it!

Solution

Here's the fixed code:

That's it. That fixes our "oddity", our "cosmic ray flipping that bit to zero", our bad bad Jenkins making our perfectly good code and perfectly written test to fail.

No, there's no oddities, cosmic rays, bad Jenkins or slow CPUs (maybe sometimes there's slow CPUs). A failing test is a failing test, it's signaling something. Either the test is flawed, or there's a bug hiding somewhere behind it.

Happy coding!

Saturday, July 20, 2013

How to make an old Delphi application DEP compatible

The problem


We have an application, our core application, that used to do a couple of things that resulted on DEP violations (read more about it here Data Execution Prevention ).
This two things are:
  • Self patch framework procedures, functions or class methods
  • Generate code during runtime thru scripting engines that perform just-in-time compilation

The solution


Self patching code


Normally developers rely on self patching code when the framework they are utilizing doesn't conform to something they consider should be proper and default behavior, to extend a framework otherwise impossible to modify or to fix a bug on the framework. There might be other reasons I'm omitting here, but these are the most common I've seen. On our case, we pretty much have self-patching code that follow those three descriptions.

Most self patching code uses one variation or another of the same technique, which implies overwriting the first bytes of a procedure to perform a JMP operation of some kind to the new code that replaces the old code.

This is an example of a procedure we use to self-patch code:

procedure PatchMemory(p : Pointer; DataSize : Integer; Data : Pointer; OldData : pointer); 
{$IFNDEF DELPHIXE2}
type
  SIZE_T = DWORD;
{$ENDIF}  
var
  OldProtect : DWORD;
  BytesWritten : SIZE_T;
begin
  VirtualProtect (p, DataSize, PAGE_EXECUTE_READWRITE, OldProtect);
  Move (p^, OldData^, DataSize);
  WriteProcessMemory(GetCurrentProcess, p, Data, DataSize, BytesWritten);
  VirtualProtect (p, DataSize, OldProtect, OldProtect);
end;


The key pieces of the code above are the calls to VirtualProtect() and WriteProcessMemory(). 

Before you overwrite a piece of memory in the code segment, you MUST unprotect the memory using a call to VirtualProtect() with new protection option PAGE_EXECUTE_READWRITE.

The second thing you have to take into consideration is *how* you overwrite the memory on the code segment. I've seen implementations that simply do a move() call with the source and target memory addresses only to see the code fail with DEP violation, even when VirtualProtect() was properly called before.
It's interesting to note that on Microsoft WriteProcessMemory() spec page there's not special note to the fact it's the only way I know of to overwrite a piece of memory on the code segment without getting a DEP violation error.

With a function like the one above, as long as you follow the basic premise of unprotect the memory first and then do patch the memory using WriteProcessMemory() you will be pretty much covered for the typical issue of overwriting the code segment in any other way.


Script engine JIT compilers


Most extensible applications/frameworks rely on some kind of scripting language. From simple yet powerful "configuration" dialects to full blow scripting languages. We use a couple of scripting languages on our programs, both being dialects of Pascal. 
One of this scripting languages is newer and does its job the right way, by allocating the memory for the JIT generated code using VirtualAlloc() passing memory protection attribute PAGE_EXECUTE_READWRITE. That makes the code generated on the heap executable by simply jumping into it.

The older of our scripting engines, simply created a Delphi TMemoryStream object, and wrote into it the generated code. After it was done compiling, it tried to jump into the code generated and that of course failed with a DEP violation.
The problem in this case is that the memory allocated by the Delphi's default memory allocator doesn't use PAGE_EXECUTE_READWRITE, but PAGE_READWRITE. This is fine, and you don't want to change this default behavior

The solution for this particular scripting engine was to replace the default TMemoryStream class, which allocates normal Delphi heap memory, with a descendant of TCustomMemoryStream class which implements it's own memory allocation approach by calling VirtualAlloc() directly with memory protection attribute PAGE_EXECUTE_READWRITE.

Gist for the class: TWinVMMemoryStream


Conclusion


Just by attacking these two problems we made our application DEP compliant.
If you are having a hard time identifying where your non-compliant code might be, my suggestion will be to first try to narrow down when the violations happen.

If violations happen when starting the app, it's likely there self-patching code violations being invoked on the initialization section of unit/modules.

If it happens later down the road, once it's up and running, it's more likely there's some JIT compiler as the culprit.

Anyway, once you know what are the two tricks you have to do:
  • Always call VirtualProtect() before self-patching code, and do it using WriteProcessMemory().
  • Make sure to allocate memory for JIT generated code using VirtualAlloc() with memory protection attribute PAGE_EXECUTE_READWRITE
You will get to DEP compliance in a breeze.

Happy coding!