[PATCH 3/3] rpcrt4: Don't free server-allocated memory with the MustFree flag.
Zebediah Figura
z.figura12 at gmail.com
Mon Oct 15 13:13:51 CDT 2018
On 15/10/18 12:39, Zebediah Figura wrote:
> 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?
>
>
Actually, never mind, I just did a little more testing and found the
problem: MIDL will mark these pointers as on-stack if and only if -Oicf
mode is used, so they won't be freed by NdrPointerFree(). widl doesn't
do that.
It's still kind of weird that MIDL doesn't allocate inner pointers on
the stack in -Os mode. I guess it's just an optimization they never
implemented because they want everyone to use -Oicf mode.
More information about the wine-devel
mailing list