| From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
|---|---|
| To: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | Make formatting.c use StringInfos for output buffers |
| Date: | 2026-08-13 17:24:31 |
| Message-ID: | 3451175.1786641871@sss.pgh.pa.us |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
We've had multiple security bugs (CVE-2026-14669, CVE-2015-0241) in
formatting.c due to its habit of using output buffers of predetermined
length, which are usually much too big but sometimes not big enough.
And the path of least resistance for those security fixes was to
impose arbitrary limits on substring lengths, which is surely a wart.
I think it's time to stop the bleeding once and for all, by switching
that code over to using StringInfos for its output buffers.
That turns out to be a good deal more painful than one could wish,
because the code is messy and inconsistent about how it uses its
buffers; but detangling that seemed like a good code cleanup exercise
anyway. So attached is a patch series to accomplish this:
0001: Convert DCH_to_char() to use a StringInfo output buffer.
0002: Restructure NUM_processor() to make it clearer which
string is the output buffer.
0003: Convert NUM_processor() to use StringInfo output buffers.
0004: Try to buy back some performance (see below).
The main problem with this proposal is that it makes these functions
a little slower, apparently because calling snprintf() via
AppendStringInfo() is slower than calling it directly. After the
performance hacking in 0004, what I see is that float8_to_char
and numeric_to_number are the same speed or a little faster than
before, but timestamptz_to_char is still around 10% slower in
a tight-loop benchmark. (See drive_formatting.c, attached, for
the benchmark infrastructure.) Maybe that's okay given that the
overall effect on a complete SQL query should be far less, but
I'd still like to squeeze out a bit more speed. I don't see any
additional low-hanging fruit though.
regards, tom lane
| Attachment | Content-Type | Size |
|---|---|---|
| v1-0001-Convert-DCH_to_char-to-use-a-StringInfo-as-destin.patch | text/x-diff | 31.4 KB |
| v1-0002-Restructure-formatting.c-s-NUM_processor-to-add-s.patch | text/x-diff | 41.4 KB |
| v1-0003-Replace-formatting.c-s-fixed-size-output-buffers-.patch | text/x-diff | 25.4 KB |
| v1-0004-Micro-optimize-appendStringInfo-VA.patch | text/x-diff | 3.8 KB |
| drive_formatting.c.nocfbot | text/x-c | 3.0 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Vitaly Davydov | 2026-08-13 17:35:03 | Re: Deadlock detector fails to activate on a hot standby replica |
| Previous Message | Nitin Motiani | 2026-08-13 16:54:53 | Re: Adding pg_dump flag for parallel export to pipes |