Re: initdb SegFault

At Tue, 19 Nov 2019 12:06:50 -0500, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote in
> Andres Freund <andres(at)anarazel(dot)de> writes:
> > Agreed wrt this specific failure scenario. It does however seem not
> > great that callsites for PQExpBuffer ought to check every call for
> > allocation failures, in the general case.
>
> It is possible to check just once at the end, using the PQExpBufferBroken
> API, and I believe that libpq for instance is fairly careful about that.

FWIW, I looked though the callers of PQExpBuffer.

pqGetErrorNotice3 seems ingoring OOM on message buffer when !isError,
then sets NULL to res->errMsg. getParameterStatus doesn't check that
before use, too.

Most of the callers of PQExpBufferDataBroken use libpq_gettext("out of
memory"). And some of them do strdup(libpq_gettext()).

Not restricting to libpq functions,

dblink_connstr_check complains as "password is required" when
PQconninfoParse hits OOM.

libpqrcv_check_conninfo() will show '(null)' or maybe get SEGV on some
platforms when PQconninfoParse() hits OOM, since it uses err without
null checking. pg_basebackup, pg_dumpall and pg_isready is doing the
same thing.

> I agree that programs that just need to print something and exit could
> perhaps ask pqexpbuffer.c to handle that for them. (But initdb still
> doesn't fall in that category, because of its very nontrivial atexit
> handler :-(.)
>
> > I wonder if, for frontend paths, a simplified error handling path would
> > be worthwhile for OOM paths. Doing only a write() or such to print an
> > error message.
>
> Perhaps. You wouldn't get any translation --- but then, gettext is
> probably going to fail anyway under such conditions.

I think we should refrain from translating in the cases.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center