On 16/06/2021 13:22, Greg Nancarrow wrote:
> Hi,
>
> There's a couple of calls to GetMultiXactIdMembers() in heapam.c which
> subsequently pfree() the returned "members" pointer (pass-by-reference
> parameter) if it's non-NULL.
> However, there's an error return within GetMultiXactIdMembers() that
> returns -1 without NULLing out "members", and the callers have simply
> allocated that pointer on the stack without initializing it to NULL.
> If that error condition were to ever happen, pfree() would likely be
> called with a junk value.
> Also note that there's another error return (about 15 lines further
> down) in GetMultiXactIdMembers() that returns -1 and does NULL out
> "members", so the handling is inconsistent.
> The attached patch adds the NULLing out of the "members" pointer in
> the first error case, to fix that and guard against possible pfree()
> on error by such callers.
Thanks! Committed with a few additional cleanups.
> I also note that there are other callers which pfree() "members" based
> on the returned "nmembers" value, and this is also inconsistent.
> Some pfree() "members" if nmembers>= 0, while others pfree() it if nmembers>0.
> After looking at the code for a while, it looks like the "nmembers ==
> 0" case can't actually happen (right?). I decided not to mess with any
> of the calling code.
I added an assertion that it never returns nmembers==0.
- Heikki