c# - P/Invoke bug or am I doing it wrong? -


so, wrote 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 follows:

[dllimport("advapi32", charset = charset.unicode, setlasterror = true)] public static extern bool changeserviceconfig2w(intptr hservice, serviceconfigtype dwinfolevel, ref service_failure_actionsw lpinfo); 

serviceconfigtype enum, , particular member has value 2. service_failure_actionsw structure defined 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 buffer bit of space @ end, if first byte isn't aligned 64-bit             // boundary, can ignore first few bytes , find next 64-bit boundary.             _buffer = marshal.allochglobal(data.lpsaactions.length * actionstructuresize + 8);              data.lpsaactionsptr = _buffer;              // round next multiple of 8 64-bit-aligned pointer.             if ((data.lpsaactionsptr.toint64() & 7) != 0)             {                 data.lpsaactionsptr += 8;                 data.lpsaactionsptr -= (int)((long)data.lpsaactionsptr & ~7);             }              // copy data lpsaactions buffer.             intptr elementptr = data.lpsaactionsptr;              (int i=0; < 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 member @ end of structure doesn't appear in native structure definition, lpsaactions, simplifies use of struct , results in data being marshalled in @ end -- data underlying api ignores because assumes struct has come end in memory.)

sc_action defined this:

[structlayout(layoutkind.sequential)] struct sc_action {     public sc_action_type type;     public int delay; } 

..and sc_action_type 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 initialized 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, , last entry repeated indefinitely */ },             },     }; 

when run code within 64-bit process, works fine. when run in 32-bit process, though (actually have tested 32-bit processes on 32-bit windows installations -- don't know happens in 32-bit process on 64-bit windows installation), error_invalid_handle back.

i did bit of digging. changeserviceconfig2w api function uses stdcall calling convention, means when first opcode in function execute, stack should consist of:

  • (dword ptr) return address
  • (dword ptr) service handle
  • (dword) service config type (2)
  • (dword ptr) pointer failure actions structure

however, when attach native debugger 32-bit c# process , put breakpoint on first instruction of changeserviceconfig2w (technically first instruction of _changeserviceconfig2wstub@12), find stack consists of:

  • (dword ptr) return address
  • (dword ptr) value 0x0000afc8
  • (dword) value 0x00000001, not 2 expected
  • (dword ptr) valid pointer failure actions structure

i confirmed simple c++ application second , third dwords off of [esp] should service handle , constant value 2. tried various alternative p/invoke declarations, including using int instead of intptr , serviceconfigtype first 2 arguments, not other behaviour.

finally, changed third parameter, ref service_failure_actionsw, take intptr directly instead, , manually marshalled failureactionsstruct using marshal.structuretoptr block allocated marshal.allochglobal. declaration, first , second arguments marshal correctly.

so, question is, doing wrong in way declared changeserviceconfig2w function, account first 2 arguments not marshalling correctly? likelihood seems small, can't shake possibility i'm running actual bug in p/invoke (specifically marshaler) here.

curiously, dword value 0x0000afc8 45,000 ms delay 1 of sc_action structures within failureactionsstructure passing in. however, last member of instance, , 0x00000001 follows 0x0000afc8 in stack not type of following sc_action. if were, cannot see cause values written parameters area p/invoke call. if serializing entire structure wrong place in memory , overwriting part of stack, wouldn't cause memory corruption , terminate process?

i confused. :-)

i believe have figured out going on. in opinion, is bug in p/invoke marshaler, wouldn't surprised if microsoft's official line were, "this behaviour design."

when configure array marshaled within struct, options constrained. can tell, can use unmanagedtype.safearray of variant primitive types, or can use unmanagedtype.byvalarray, requires specify sizeconst -- is, marshaler supports embedded arrays same length.

the marshal.sizeof function, then, when computing size of structure, counts size of array sizeconst * marshal.sizeof(arrayelementtype). case, regardless of actual size of array instance points to.

the bug appears marshaler copies elements in array, if number greater sizeconst. so, in situation, if set sizeconst 1, supply array 6 elements, allocates memory based on marshal.sizeof, allocates single slot array data, , proceeds marshal 6 elements, writing past end of buffer , corrupting memory.

the reason stack slots parameters corrupted in manner can explained marshaler allocating memory serialization on stack. overflowing end of buffer, overwrote data further stack, including spot where, ultimately, return address gets written , first 2 arguments had been placed slots on stack. after marshaling operation, wrote pointer stack buffer third argument, explaining why third argument's value was, in fact, valid pointer data structure.

i lucky, particular configuration, end of 6th element occurred before corrupting other elements further stack, in first 2 arguments changeserviceconfig2w being damaged -- code able continue execute after changeserviceconfig2w returned error handle being invalid. larger array, or simpler function buffer allocated marshaler closer end of stack frame, have overwritten important data further stack , resulted in executionengineexception @gserg saw.

on 64-bit systems, there must more free space on stack -- 1 thing, pointers 64 bits wide, spaces things out. writing past end of buffer, then, isn't getting far stack , isn't managing corrupt first or second argument changeserviceconfig2w. how code worked in initial testing , appeared correct.

in opinion, bug in marshaler; has enough information avoid corrupting memory (just don't marshal more sizeconst elements, because that's memory have allocated!), goes ahead , writes past end of allocated buffer anyway. can see converse philosophy, is, "if you've told marshaler sizeconst 1, don't supply array more 1 element." there no clear warning doing can corrupt execution environment in of documentation read. given lengths .net goes to avoid type of corruption, have think of bug in marshaler.

i have gotten code working updating datalock class, temporarily prepares lpsaactions data pointer-to-array (which changeserviceconfig2w requires , default p/invoke marshaler not appear support), stash real lpsaactions array , replace dummy 1-element array. prevents marshaler marshaling more 1 element, , memory corruption not occur. when datalock object disposed, restores lpsaactions previous value.

i have placed (working) code public github repo, along identical c++ version used compare state of registers , stack code flow entered changeserviceconfig2w function during diagnosis:

to reproduce problem seeing, comment out these lines windowsapi/service_failure_actionsw.cs:

            // replace lpsaactions array dummy contains 1 element, otherwise p/invoke marshaller             // allocate buffer of size 1 , write lpsaactions.length items , corrupt memory.             _originalactionsarray = data.lpsaactions;              data.lpsaactions = new sc_action[1]; 

then, run program 32-bit process. (if on 64-bit operating system, likely, you'll need adjust build configuration either specifies "prefer 32-bit" or targets "x86" platform directly.)

microsoft may or may not become aware of , fix bug. @ least, of microsoft update documentation unmanagedtype.byvalarray include warning possible scenario -- i'm not sure how convey them. given current releases of .net have it, though, think best avoid supplying arrays length not equal sizeconst when marshaling structures unmanaged code. :-)


Comments