From: Tom Lane <tgl@sss.pgh.pa.us>
To: Thomas Munro <thomas.munro@enterprisedb.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Allowing printf("%m") only where it actually works
Date: Sat, 26 May 2018 20:38:03 -0400
Message-ID: <1673.1527381483@sss.pgh.pa.us> (raw)
In-Reply-To: <CAEepm=0oVAR8AOKjK-Yq34KoAMXqZbTHsF_rpZqA5skDXe95Rg@mail.gmail.com>
References: <2975.1526862605@sss.pgh.pa.us>
<CAEepm=29o4wi5WEBFfwhBUuiYSymbTcORw_cvdaaxPPoAkrsvA@mail.gmail.com>
<14176.1526874887@sss.pgh.pa.us>
<24986.1526877413@sss.pgh.pa.us>
<CAEepm=2-7qBw6U-vy2OgL=Z=J6VeOzPS5jsK2PwmG0PWTDv1cg@mail.gmail.com>
<31054.1526918600@sss.pgh.pa.us>
<6025.1527351693@sss.pgh.pa.us>
<CAEepm=0oVAR8AOKjK-Yq34KoAMXqZbTHsF_rpZqA5skDXe95Rg@mail.gmail.com>
Thomas Munro <thomas.munro@enterprisedb.com> writes:
> On Sun, May 27, 2018 at 4:21 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> (Basically what this would protect against is elog_start changing errno,
>> which it doesn't.)
> Hmm. It looks like errstart() preserves errno to protect %m not from
> itself, but from the caller's other arguments to the elog facility.
> That seems reasonable, but do we really need to prohibit direct use of
> errno in expressions? The only rogue actor likely to trash errno is
> you, the caller. I mean, if you call elog(LOG, "foo %d %d", errno,
> fsync(bar)) it's obviously UB and your own fault, but who would do
> anything like that? Or maybe I misunderstood the motivation.
Right, the concern is really about things like
elog(..., f(x), strerror(errno));
If f() trashes errno --- perhaps only some of the time --- then this
is problematic. It's especially problematic because whether f() is
evaluated before or after strerror() is platform-dependent. So even
if the original author had tested things thoroughly, it might break
for somebody else.
The cases in exec.c all seem safe enough, but we have lots of examples
in the backend where we call nontrivial functions in the arguments of
elog/ereport. It doesn't take much to make one nontrivial either. If
memory serves, malloc() can trash errno on some platforms such as macOS,
so even just a palloc creates a hazard of a hard-to-reproduce problem.
At least in the case of ereport, all it takes to create a hazard is
more than one sub-function, eg this is risky:
ereport(..., errmsg(..., strerror(errno)), errdetail(...));
because errdetail() might run first and malloc some memory for its
constructed string.
So I think a blanket policy of "don't trust errno within the arguments"
is a good idea, even though it might be safe to violate it in the
existing cases in exec.c.
regards, tom lane
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, thomas.munro@enterprisedb.com, pgsql-hackers@lists.postgresql.org
Subject: Re: Allowing printf("%m") only where it actually works
In-Reply-To: <1673.1527381483@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox