So, I wrote the following code:
using (var serviceController = new ServiceController(serviceName))
{
var serviceHandle = serviceController.ServiceHandle;
using (failureActionsStructure.Lock())
{
success = NativeMethods.ChangeServiceConfig2W(
serviceHandle,
ServiceConfigType.SERVICE_CONFIG_FAILURE_ACTIONS,
ref failureActionsStructure);
if (!success)
throw new Win32Exception();
}
}
The P/Invoke declaration is as follows:
[DllImport("advapi32", CharSet = CharSet.Unicode, SetLastError = true)]
public static extern bool ChangeServiceConfig2W(IntPtr hService, ServiceConfigType dwInfoLevel, ref SERVICE_FAILURE_ACTIONSW lpInfo);
ServiceConfigType is just an enum, and this particular member has value 2. The SERVICE_FAILURE_ACTIONSW structure is defined as follows:
[StructLayout(LayoutKind.Sequential, CharSet = CharSet.Unicode)]
struct SERVICE_FAILURE_ACTIONSW
{
public int dwResetPeriod;
public string lpRebootMsg;
public string lpCommand;
public int cActions;
public IntPtr lpsaActionsPtr;
[MarshalAs(UnmanagedType.ByValArray, SizeConst = 1)]
public SC_ACTION[] lpsaActions;
class DataLock : IDisposable
{
IntPtr _buffer;
public DataLock(ref SERVICE_FAILURE_ACTIONSW data)
{
int actionStructureSize = Marshal.SizeOf(typeof(SC_ACTION));
// Allocate a buffer with a bit of extra space at the end, so that if the first byte isn't aligned to a 64-bit
// boundary, we can simply ignore the first few bytes and find the next 64-bit boundary.
_buffer = Marshal.AllocHGlobal(data.lpsaActions.Length * actionStructureSize + 8);
data.lpsaActionsPtr = _buffer;
// Round up to the next multiple of 8 to get a 64-bit-aligned pointer.
if ((data.lpsaActionsPtr.ToInt64() & 7) != 0)
{
data.lpsaActionsPtr += 8;
data.lpsaActionsPtr -= (int)((long)data.lpsaActionsPtr & ~7);
}
// Copy the data from lpsaActions into the buffer.
IntPtr elementPtr = data.lpsaActionsPtr;
for (int i=0; i < data.lpsaActions.Length; i++, elementPtr += actionStructureSize)
Marshal.StructureToPtr(data.lpsaActions[i], elementPtr, fDeleteOld: false);
}
public void Dispose()
{
Marshal.FreeHGlobal(_buffer);
}
}
internal IDisposable Lock()
{
return new DataLock(ref this);
}
}
(This type has an extra member at the end of the structure that doesn't appear in the native structure definition, lpsaActions, which simplifies the use of this struct and results only in extra data being marshalled in at the end -- data that the underlying API simply ignores because it assumes the struct has already come to an end in memory.)
SC_ACTION is defined like this:
[StructLayout(LayoutKind.Sequential)]
struct SC_ACTION
{
public SC_ACTION_TYPE Type;
public int Delay;
}
..and SC_ACTION_TYPE is a straightforward enum:
enum SC_ACTION_TYPE
{
SC_ACTION_NONE = 0,
SC_ACTION_RESTART = 1,
SC_ACTION_REBOOT = 2,
SC_ACTION_RUN_COMMAND = 3,
}
The structure I'm passing in is initialized like this:
var failureActionsStructure =
new SERVICE_FAILURE_ACTIONSW()
{
dwResetPeriod = 60000, // 60 seconds
lpRebootMsg = "",
lpCommand = "",
cActions = 6,
lpsaActions =
new SC_ACTION[]
{
new SC_ACTION() { Type = SC_ACTION_TYPE.SC_ACTION_RESTART /* 1 */, Delay = 5000 /* 5 seconds */ },
new SC_ACTION() { Type = SC_ACTION_TYPE.SC_ACTION_RESTART /* 1 */, Delay = 15000 /* 15 seconds */ },
new SC_ACTION() { Type = SC_ACTION_TYPE.SC_ACTION_RESTART /* 1 */, Delay = 25000 /* 25 seconds */ },
new SC_ACTION() { Type = SC_ACTION_TYPE.SC_ACTION_RESTART /* 1 */, Delay = 35000 /* 35 seconds */ },
new SC_ACTION() { Type = SC_ACTION_TYPE.SC_ACTION_RESTART /* 1 */, Delay = 45000 /* 45 seconds */ },
new SC_ACTION() { Type = SC_ACTION_TYPE.SC_ACTION_NONE /* 0 */, Delay = 0 /* immediate, and this last entry is then repeated indefinitely */ },
},
};
When I run this code within a 64-bit process, it works just fine. When I run it in a 32-bit process, though (actually I have only tested 32-bit processes on 32-bit Windows installations -- I don't know for certain what happens in a 32-bit process on a 64-bit Windows installation), I always get ERROR_INVALID_HANDLE back.
I did a bit of digging. The ChangeServiceConfig2W API function uses the stdcall calling convention, which means that when the first opcode in the function is about to execute, the stack should consist of:
However, when I attach a native debugger to my 32-bit C# process and put a breakpoint on the first instruction of ChangeServiceConfig2W (technically the first instruction of _ChangeServiceConfig2WStub@12), I find that the stack consists of:
I confirmed with a simple C++ application that the second and third DWORDs off of [ESP] should be the Service Handle and constant value 2. I tried various alternative P/Invoke declarations, including using int instead of IntPtr and ServiceConfigType for the first two arguments, but could not get any other behaviour.
Finally, I changed the third parameter, the ref SERVICE_FAILURE_ACTIONSW, to take IntPtr directly instead, and manually marshalled the failureActionsStruct using Marshal.StructureToPtr into a block allocated with Marshal.AllocHGlobal. With this declaration, the first and second arguments now marshal correctly.
So, my question is, am I doing something wrong in the way that I initially declared the ChangeServiceConfig2W function, something that could account for the first two arguments not marshalling correctly? The likelihood seems small, but I can't shake the possibility that I'm running into an actual bug in P/Invoke (specifically the marshaler) here.
Curiously, the DWORD value 0x0000AFC8 is the 45,000 ms Delay from one of the SC_ACTION structures within failureActionsStructure I am passing in. However, it is the last member of that instance, and the 0x00000001 that follows 0x0000AFC8 in the stack would not be the Type of the following SC_ACTION. Even if it were, I cannot see what would cause those values to be written specifically to the parameters area for the P/Invoke call. If it were serializing the entire structure to the wrong place in memory and overwriting part of the stack, wouldn't that cause memory corruption and probably terminate the process?
I am confused. :-)
I believe I have figured out what is going on. In my opinion, it is a bug in the P/Invoke marshaler, but I wouldn't be surprised if Microsoft's official line were, "This behaviour is by design."
When you configure an array to be marshaled within a struct, your options are very constrained. From what I can tell, you can use UnmanagedType.SafeArray for any of the VARIANT primitive types, or you can use UnmanagedType.ByValArray, which requires you to specify SizeConst -- that is, the marshaler supports only embedded arrays that are always exactly the same length.
The Marshal.SizeOf function, then, when computing the size of the structure, counts the size of the array as SizeConst * Marshal.SizeOf(arrayElementType). This is always the case, regardless of the actual size of the array an instance points to.
The bug appears to be that the marshaler always copies all elements in the array, even if that number is greater than SizeConst. So, as in my situation, if you set SizeConst to 1, but supply an array with 6 elements, then it allocates memory based on Marshal.SizeOf, which allocates a single slot for array data, and then proceeds to marshal all 6 elements, writing past the end of the buffer and corrupting memory.
The reason the stack slots for parameters were corrupted in this manner can be explained only by the marshaler allocating memory for this serialization on the stack. By overflowing the end of that buffer, it then overwrote data further up the stack, including the spot where, ultimately, the return address gets written and the first two arguments that had already been placed into their slots on the stack. After this marshaling operation, it then wrote a pointer to that stack buffer into the third argument, explaining why the third argument's value was, in fact, a valid pointer to the data structure.
I was lucky, with my particular configuration, that the end of the 6th element occurred before corrupting other elements further up the stack, in that only the first two arguments to ChangeServiceConfig2W were being damaged -- code was able to continue to execute after ChangeServiceConfig2W returned its error about the handle being invalid. With a larger array, or a simpler function where the buffer allocated by the marshaler was closer to the end of the stack frame, it could very well have overwritten important data further up the stack and resulted in an ExecutionEngineException as @GSerg saw.
On 64-bit systems, there must be more free space on the stack -- for one thing, all pointers are now 64 bits wide, so that spaces things out. Writing past the end of the buffer, then, isn't getting as far up the stack and isn't managing to corrupt the first or second argument to ChangeServiceConfig2W. This is how this code worked in initial testing and appeared to be correct.
In my opinion, this is a bug in the marshaler; it has enough information to avoid corrupting memory (just don't marshal more than SizeConst elements, because that's all the memory you have allocated!), but it goes ahead and writes past the end of the allocated buffer anyway. I can see the converse philosophy, which is, "If you've told the marshaler SizeConst is 1, then don't supply an array with more than 1 element." But there is no clear warning that doing so can corrupt the execution environment in any of the documentation that I read. Given the lengths that .NET goes to to avoid that type of corruption, I have to think of this as a bug in the marshaler.
I have gotten my code working by updating the DataLock class, which temporarily prepares the lpsaActions data as a pointer-to-array (which ChangeServiceConfig2W requires and the default P/Invoke marshaler does not appear to support), to stash the real lpsaActions array and replace it with a dummy 1-element array. This prevents the marshaler from then marshaling more than 1 element, and the memory corruption does not occur. When the DataLock object is Disposed, it restores lpsaActions to its previous value.
I have placed this (working) code into a public GitHub repo, along with an essentially identical C++ version that I used to compare the state of registers and the stack as code flow entered the ChangeServiceConfig2W function during my diagnosis:
To reproduce the problem I was seeing, comment out these lines from WindowsAPI/SERVICE_FAILURE_ACTIONSW.cs:
// Replace the lpsaActions array with a dummy that contains only one element, otherwise the P/Invoke marshaller
// will allocate a buffer of size 1 and then write lpsaActions.Length items to it and corrupt memory.
_originalActionsArray = data.lpsaActions;
data.lpsaActions = new SC_ACTION[1];
Then, run the program as a 32-bit process. (If you are on a 64-bit operating system, which is likely, you'll need to adjust the build configuration so that it either specifies "Prefer 32-bit" or targets the "x86" platform directly.)
Microsoft may or may not become aware of and fix this bug. At the very least, it would be good of Microsoft to update the documentation for UnmanagedType.ByValArray to include a warning about this possible scenario -- I'm not sure how to convey that to them. Given that current releases of .NET have it, though, I think it is best to simply avoid supplying arrays whose length is not exactly equal to SizeConst when marshaling structures to unmanaged code. :-)