Daniel Verite wrote: > My tests are OK too but I see an issue with the code in > enlargeStringInfo(), regarding integer overflow. > The bit of comment that says: > > Note we are assuming here that limit <= INT_MAX/2, else the above > loop could overflow. > > is obsolete, it's now INT_MAX instead of INT_MAX/2.
I would keep this comment but use UINT_MAX/2 instead. > There's a related problem here: > newlen = 2 * str->maxlen; > while (needed > newlen) > newlen = 2 * newlen; > str->maxlen is an int going up to INT_MAX so [2 * str->maxlen] now > *will* overflow when [str->maxlen > INT_MAX/2]. > Eventually it somehow works because of this: > if (newlen > limit) > newlen = limit; > but newlen is wonky (when resulting from int overflow) > before being brought back to limit. Yeah, you're right. We also need to cast "needed" to Size in the while test; and the repalloc_huge() call no longer needs a cast. I propose the attached. Not sure if we also need to cast the assignment to str->maxlen in the last line. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
commit 1327c2eea9c7ca074d88bb167bd4c35338d2de0b[m Author: Alvaro Herrera <alvhe...@alvh.no-ip.org> AuthorDate: Tue Jan 10 02:46:42 2017 -0300 CommitDate: Tue Jan 10 02:46:42 2017 -0300 Fix overflow check in StringInfo A thinko I introduced in fa2fa9955280. Also, amend a similarly broken comment. Report and patch by Daniel Vérité. Discussion: https://postgr.es/m/1706e85e-60d2-494e-8a64-9af1e1b21...@manitou-mail.org diff --git a/src/backend/lib/stringinfo.c b/src/backend/lib/stringinfo.c index bdc204e..11d751a 100644 --- a/src/backend/lib/stringinfo.c +++ b/src/backend/lib/stringinfo.c @@ -313,19 +313,19 @@ enlargeStringInfo(StringInfo str, int needed) * for efficiency, double the buffer size each time it overflows. * Actually, we might need to more than double it if 'needed' is big... */ - newlen = 2 * str->maxlen; - while (needed > newlen) + newlen = 2 * (Size) str->maxlen; + while ((Size) needed > newlen) newlen = 2 * newlen; /* * Clamp to the limit in case we went past it. Note we are assuming here - * that limit <= INT_MAX/2, else the above loop could overflow. We will + * that limit <= UINT_MAX/2, else the above loop could overflow. We will * still have newlen >= needed. */ if (newlen > limit) newlen = limit; - str->data = (char *) repalloc_huge(str->data, (Size) newlen); + str->data = (char *) repalloc_huge(str->data, newlen); str->maxlen = newlen; }
-- Sent via pgsql-hackers mailing list (firstname.lastname@example.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers