From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com> |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Code quality issues in ICU patch |
Date: | 2017-06-24 15:51:41 |
Message-ID: | 22161.1498319501@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com> writes:
> On 6/23/17 12:31, Tom Lane wrote:
>> icu_to_uchar() and icu_from_uchar(), and perhaps other places, are
>> touchingly naive about integer overflow hazards in buffer size
>> calculations.
> Here is a patch that should address this.
Ah, I was about to suggest the same thing, but I was coming at it from
the standpoint of not requiring buffers several times larger than
necessary, which could in itself cause avoidable palloc failures.
I was going to suggest a small variant actually: run the conversion
function an extra time only if the string is long enough to make the
space consumption interesting, say
if (nbytes < 1024)
{
/* if it's short, feel free to waste a bit of space */
len_uchar = 2 * nbytes + 1; /* max length per docs */
}
else
{
/* calculate exact space needed */
status = U_ZERO_ERROR;
len_uchar = ucnv_toUChars(icu_converter, NULL, 0,
buff, nbytes, &status);
if (U_FAILURE(status) && status != U_BUFFER_OVERFLOW_ERROR)
...
}
*buff_uchar = palloc(len_uchar * sizeof(**buff_uchar));
In this way the extra cycles would seldom be paid in practice.
> (I don't think the overruns were exploitable. You'd just get a buffer
> overflow error from the ucnv_* function.)
Hm, good point. But we might still hit avoidable failures with strings
that are a significant fraction of 1GB.
regards, tom lane
From | Date | Subject | |
---|---|---|---|
Next Message | Joe Conway | 2017-06-24 17:41:27 | Re: FIPS mode? |
Previous Message | Simon Riggs | 2017-06-24 14:36:11 | Re: Causal reads take II |