[PATCH 3/3] rpcrt4: Don't free server-allocated memory with the MustFree flag.

[Zebediah Figura]

On 15/10/18 04:18, Huw Davies wrote:
> On Sat, Oct 13, 2018 at 05:46:44PM -0500, Zebediah Figura wrote:
>> Since it will have already been freed in the MUSTFREE pass.
>>
>> Signed-off-by: Zebediah Figura <z.figura12 at gmail.com>
>> ---
>>  dlls/rpcrt4/ndr_stubless.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/dlls/rpcrt4/ndr_stubless.c b/dlls/rpcrt4/ndr_stubless.c
>> index c2f260c..768bdc1 100644
>> --- a/dlls/rpcrt4/ndr_stubless.c
>> +++ b/dlls/rpcrt4/ndr_stubless.c
>> @@ -1215,7 +1215,7 @@ static LONG_PTR *stub_do_args(MIDL_STUB_MESSAGE *pStubMsg,
>>              }
>>              break;
>>          case STUBLESS_FREE:
>> -            if (params[i].attr.ServerAllocSize)
>> +            if (params[i].attr.ServerAllocSize && !params[i].attr.MustFree)
>>              {
>>                  HeapFree(GetProcessHeap(), 0, *(void **)pArg);
>>              }
> 
> Do we need to free in the IsSimpleRef case (like the else block
> below)?  In this case the freer won't see the top-level ptr, though
> it may not be an issue with ServerAllocSize.
> 
> Huw.
> 
> 

Hmm, no, you're right, we do need to free in that case. So I think the
condition should be (ServerAllocSize && (!MustFree || IsSimpleRef)).

What confuses me about the whole ordeal is that in theory,
ServerAllocSize represents storage space that should exist on the stub's
stack. MSDN states this:

"The ServerAllocSize bits are nonzero if the parameter is [out], [in],
or [in,out] pointer to pointer, or pointer to enum16, and will be
initialized on the server interpreter's stack, rather than using a call
to I_RpcAllocate."

and -Os mode stubs back this up — but only in some cases. Namely, a
parameter like "[out] mystruct_t *x" will have a server size of 16 bits,
and the inline stub will include a generated "MYSTRUCT _xM" which the
server function is called with. However, in the case that's causing the
crash for me, i.e. where IsSimpleRef is *not* set, e.g. any double
pointer, the -Os mode stubs don't allocate stack space for the inner
pointer, and just calling the type's freer — i.e. NdrPointerFree() —
would incorrectly free that stack space if it were allocated. The
effect, then, is that ServerAllocSize seems to have to be entirely
ignored if IsSimpleRef isn't also set, which is weird. Am I missing
something here?