From: Zebediah Figura Subject: Re: [PATCH 3/3] rpcrt4: Don't free server-allocated memory with the MustFree flag. Message-Id: Date: Mon, 15 Oct 2018 12:39:23 -0500 In-Reply-To: <20181015091852.GN31910@merlot.physics.ox.ac.uk> References: <1539470804-32048-1-git-send-email-z.figura12@gmail.com> <1539470804-32048-3-git-send-email-z.figura12@gmail.com> <20181015091852.GN31910@merlot.physics.ox.ac.uk> 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 >> --- >> 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?