| From: | Michael Paquier <michael(at)paquier(dot)xyz> |
|---|---|
| To: | Keyerror Smart <smartkeyerror(at)gmail(dot)com> |
| Cc: | Tender Wang <tndrwang(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: [BUG] hstore integer overflow when constructing large values |
| Date: | 2026-08-16 06:45:08 |
| Message-ID: | aoFcc-__HMsJPvXp@paquier.xyz |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Sat, Aug 15, 2026 at 02:52:48PM +0800, Keyerror Smart wrote:
> I agree that a regression test isn't practical here, since triggering
> the overflow requires more than 2GB of input; that's consistent with
> the existing limits (e.g. HSTORE_MAX_KEY_LEN) not being exercised in
> the tests either.
+/*
+ * Add the string-data length of a pair to *buflen, checking for overflow.
+ * Individual keys and values are each limited to HENTRY_POSMASK bytes, but
+ * their combined length can exceed the range of int32.
+ */
+static void
+hstoreAddPairLen(int32 *buflen, const Pairs *pair)
+{
+ Size pairlen;
+
+ pairlen = pair->keylen + (pair->isnull ? 0 : pair->vallen);
+ if (pairlen > MaxAllocSize - (Size) *buflen)
+ ereport(ERROR,
+ (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
+ errmsg("hstore is too large")));
+ *buflen += pairlen;
+}
Switching to Size is what we should do, but the proposed patch is
doing it incorrectly and is actually incomplete. Let's rework all
this code so as we do not rely on int32 anymore for the calculated
length passed down to palloc(), and rely instead on add_size(), as
controlled by palloc.h and mcxt.c. What I mean here is to think more
deeply through this code rather than try to plug in weirdly one aspect
of the failures. (Bonus points: add_size() handles overflows and
complains about them.)
I doubt that I would backpatch any of that. There is nothing critical
as far as I understand, still it's a nice long-term improvement of the
allocation logic to use a Size where we can, going through the
palloc() overflow checks.
--
Michael
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Michael Paquier | 2026-08-16 06:56:03 | Re: [PATCH] Fix compilation of nodeMergejoin.c with EXEC_MERGEJOINDEBUG |
| Previous Message | Michael Paquier | 2026-08-16 06:31:34 | Re: GiST wal_consistency_checking issue |