From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Daniel Verite <daniel@manitou-mail.org>
Cc: Tomas Vondra <tomas.vondra@2ndquadrant.com>
Cc: Craig Ringer <craig@2ndquadrant.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: Jim Nasby <Jim.Nasby@bluetreble.com>
Cc: Ronan Dunklau <ronan.dunklau@dalibo.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: pg_dump / copy bugs with "big lines" ?
Date: Tue, 10 Jan 2017 02:50:57 -0300
Message-ID: <20170110055057.utnaxegvbuwslpex@alvherre.pgsql> (raw)
In-Reply-To: <1706e85e-60d2-494e-8a64-9af1e1b2186e@manitou-mail.org>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-hackers>
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 <alvherre@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-9af1e1b2186e@manitou-mail.orgdiff --git a/src/backend/lib/stringinfo.c b/src/backend/lib/stringinfo.cindex 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 (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackersAttachments:
[text/plain] fix-stringinfo.patch (1.4K, ../20170110055057.utnaxegvbuwslpex@alvherre.pgsql/2-fix-stringinfo.patch)
download | inline diff:
commit 1327c2eea9c7ca074d88bb167bd4c35338d2de0b[m
Author: Alvaro Herrera <alvherre@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-9af1e1b2186e@manitou-mail.org
diff --git a/src/backend/lib/stringinfo.c b/src/backend/lib/stringinfo.cindex 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;
}
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: alvherre@2ndquadrant.com, daniel@manitou-mail.org, tomas.vondra@2ndquadrant.com, craig@2ndquadrant.com, tgl@sss.pgh.pa.us, robertmhaas@gmail.com, Jim.Nasby@bluetreble.com, ronan.dunklau@dalibo.com
Subject: Re: pg_dump / copy bugs with "big lines" ?
In-Reply-To: <20170110055057.utnaxegvbuwslpex@alvherre.pgsql>
* 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