Re: Non-robustness in pmsignal.c

Andres Freund <andres@anarazel.de>

From: Andres Freund <andres@anarazel.de>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@lists.postgresql.org
Date: 2022-10-08T17:32:36Z
Lists: pgsql-hackers
Hi,

On 2022-10-08 13:15:07 -0400, Tom Lane wrote:
> I wrote:
> > Andres Freund <andres@anarazel.de> writes:
> >> Only PM_CHILD_ACTIVE and PM_CHILD_WALSENDER though. We could afford another
> >> MaxLivePostmasterChildren() sized array...
> 
> > Oh, I see what you mean --- one private and one public array.
> > Maybe that makes more sense than what I did, not sure.
> 
> Yeah, that's definitely a better way.  I'll push this after the
> release freeze lifts.

Cool, thanks for exploring.


>  /*
>   * Signal handler to be notified if postmaster dies.
>   */
> @@ -142,7 +152,25 @@ PMSignalShmemInit(void)
>  	{
>  		/* initialize all flags to zeroes */
>  		MemSet(unvolatize(PMSignalData *, PMSignalState), 0, PMSignalShmemSize());
> -		PMSignalState->num_child_flags = MaxLivePostmasterChildren();
> +		num_child_inuse = MaxLivePostmasterChildren();
> +		PMSignalState->num_child_flags = num_child_inuse;
> +
> +		/*
> +		 * Also allocate postmaster's private PMChildInUse[] array.  We
> +		 * might've already done that in a previous shared-memory creation
> +		 * cycle, in which case free the old array to avoid a leak.  (Do it
> +		 * like this to support the possibility that MaxLivePostmasterChildren
> +		 * changed.)  In a standalone backend, we do not need this.
> +		 */
> +		if (PostmasterContext != NULL)
> +		{
> +			if (PMChildInUse)
> +				pfree(PMChildInUse);
> +			PMChildInUse = (bool *)
> +				MemoryContextAllocZero(PostmasterContext,
> +									   num_child_inuse * sizeof(bool));
> +		}
> +		next_child_inuse = 0;
>  	}
>  }

When can PostmasterContext be NULL here, and why can we just continue without
(re-)allocating PMChildInUse?

Greetings,

Andres Freund



Commits

  1. Harden pmsignal.c against clobbered shared memory.