From: Huw Davies <huw@codeweavers.com>
Subject: Re: [PATCH 3/3] rpcrt4: Don't free server-allocated memory with the MustFree flag.
Message-Id: <20181016074845.GA10558@merlot.physics.ox.ac.uk>
Date: Tue, 16 Oct 2018 08:48:45 +0100
In-Reply-To: <259baae8-6e10-a468-b8ea-63c89b2242cd@gmail.com>
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> <ab36c7e4-06cb-a49e-3040-1ed459df7b27@gmail.com> <259baae8-6e10-a468-b8ea-63c89b2242cd@gmail.com>

On Mon, Oct 15, 2018 at 01:13:51PM -0500, Zebediah Figura wrote:
> On 15/10/18 12:39, Zebediah Figura wrote:
> > 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.

Yes, it's annoying that the format string changes across the different
modes---I've been bitten by that in the past.

Huw.