From 92e20d4a4382810a30bdb394473d6ee891edab73 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Tue, 18 Aug 2026 13:44:04 +0900 Subject: [PATCH] hstore: Rework module to use Size and {add,mul}_size for allocation sizes hstore has been relying on a set of int/int32 to count the total length of all the keys and values constructed, relying on the maximum values of the arguments rather than proper bound checks. As proved in the reported thread, these counters could overflow and wrap, leading to incorrect allocations. This commit replaces the int/int32 values with a set of Size values, with add_size() and mul_size() in charge of checking for overflows when constructing hstore values before doing any allocation. The changes that matter in terms of the report are mostly in hstore_io.c. hstore_compat.c is adjusted for consistency, to keep the allocation maths uniform across the module. This is an old bug. A backpatch should perhaps be done but I cannot get excited about a change like that in stable branches. Reported-by: Man Zeng Author: Tender Wang Author: Michael Paquier Discussion: https://postgr.es/m/CAHewXNmfD6-9MDxgpRfTzjVj0mhBw3JMoNOHeTiPOjZGhNzd=g@mail.gmail.com --- contrib/hstore/hstore.h | 35 ++++++++----- contrib/hstore/hstore_compat.c | 29 +++++------ contrib/hstore/hstore_io.c | 60 ++++++++++++++++------- contrib/hstore/hstore_op.c | 12 ++--- contrib/hstore/hstore_subs.c | 4 +- contrib/hstore_plperl/hstore_plperl.c | 6 +-- contrib/hstore_plpython/hstore_plpython.c | 6 +-- 7 files changed, 92 insertions(+), 60 deletions(-) diff --git a/contrib/hstore/hstore.h b/contrib/hstore/hstore.h index 897af244a42..39cbe4d1010 100644 --- a/contrib/hstore/hstore.h +++ b/contrib/hstore/hstore.h @@ -62,14 +62,23 @@ typedef struct #define HS_SETCOUNT(hsp_,c_) ((hsp_)->size_ = (c_) | HS_FLAG_NEWVERSION) -/* - * "x" comes from an existing HS_COUNT() (as discussed, <= INT_MAX/24) or a - * Pairs array length (due to MaxAllocSize, <= INT_MAX/40). "lenstr" is no - * more than INT_MAX, that extreme case arising in hstore_from_arrays(). - * Therefore, this calculation is limited to about INT_MAX / 5 + INT_MAX. - */ #define HSHRDSIZE (sizeof(HStore)) -#define CALCDATASIZE(x, lenstr) ( (x) * 2 * sizeof(HEntry) + HSHRDSIZE + (lenstr) ) + +/* + * "x" is a pair count, coming from an existing HS_COUNT() (as discussed, <= + * INT_MAX/24) or a Pairs array length (due to MaxAllocSize, <= INT_MAX/40). + * "lenstr" is no more than INT_MAX, that extreme case arising in + * hstore_from_arrays(). Therefore, this calculation should be limited to + * about INT_MAX / 5 + INT_MAX. + */ +static inline Size +hstoreCalcDataSize(Size x, Size lenstr) +{ + Size entrysize = mul_size(x, 2 * sizeof(HEntry)); + Size total = add_size(HSHRDSIZE, lenstr); + + return add_size(entrysize, total); +} /* note multiple evaluations of x */ #define ARRPTR(x) ( (HEntry*) ( (HStore*)(x) + 1 ) ) @@ -128,7 +137,7 @@ typedef struct /* finalize a newly-constructed hstore */ #define HS_FINALIZE(hsp_,count_,buf_,ptr_) \ do { \ - int _buflen = (ptr_) - (buf_); \ + Size _buflen = (ptr_) - (buf_); \ if ((count_)) \ ARRPTR(hsp_)[0].entry |= HENTRY_ISFIRST; \ if ((count_) != HS_COUNT((hsp_))) \ @@ -136,14 +145,14 @@ typedef struct HS_SETCOUNT((hsp_),(count_)); \ memmove(STRPTR(hsp_), (buf_), _buflen); \ } \ - SET_VARSIZE((hsp_), CALCDATASIZE((count_), _buflen)); \ + SET_VARSIZE((hsp_), hstoreCalcDataSize((count_), _buflen)); \ } while (0) /* ensure the varlena size of an existing hstore is correct */ #define HS_FIXSIZE(hsp_,count_) \ do { \ - int bl = (count_) ? HSE_ENDPOS(ARRPTR(hsp_)[2*(count_)-1]) : 0; \ - SET_VARSIZE((hsp_), CALCDATASIZE((count_),bl)); \ + Size bl = (count_) ? HSE_ENDPOS(ARRPTR(hsp_)[2*(count_)-1]) : 0; \ + SET_VARSIZE((hsp_), hstoreCalcDataSize((count_),bl)); \ } while (0) /* DatumGetHStoreP includes support for reading old-format hstore values */ @@ -168,8 +177,8 @@ typedef struct bool needfree; /* need to pfree the value? */ } Pairs; -extern PGDLLEXPORT int hstoreUniquePairs(Pairs *a, int32 l, int32 *buflen); -extern PGDLLEXPORT HStore *hstorePairs(Pairs *pairs, int32 pcount, int32 buflen); +extern PGDLLEXPORT int hstoreUniquePairs(Pairs *a, int32 l, Size *buflen); +extern PGDLLEXPORT HStore *hstorePairs(Pairs *pairs, int32 pcount, Size buflen); extern PGDLLEXPORT size_t hstoreCheckKeyLen(size_t len); extern PGDLLEXPORT size_t hstoreCheckValLen(size_t len); diff --git a/contrib/hstore/hstore_compat.c b/contrib/hstore/hstore_compat.c index 3a9f7f45cb7..1e2db807edf 100644 --- a/contrib/hstore/hstore_compat.c +++ b/contrib/hstore/hstore_compat.c @@ -90,8 +90,8 @@ * This is the structure used for entries in the old contrib/hstore * implementation. Notice that this is the same size as the new entry * (two 32-bit words per key/value pair) and that the header is the - * same, so the old and new versions of ARRPTR, STRPTR, CALCDATASIZE - * etc. are compatible. + * same, so the old and new versions of ARRPTR, STRPTR, + * hstoreCalcDataSize etc. are compatible. * * If the above statement isn't true on some bizarre platform, we're * a bit hosed. @@ -123,8 +123,8 @@ hstoreValidNewFormat(HStore *hs) { int count = HS_COUNT(hs); HEntry *entries = ARRPTR(hs); - int buflen = (count) ? HSE_ENDPOS(entries[2 * (count) - 1]) : 0; - int vsize = CALCDATASIZE(count, buflen); + Size buflen = (count) ? HSE_ENDPOS(entries[2 * (count) - 1]) : 0; + Size vsize = hstoreCalcDataSize(count, buflen); int i; if (hs->size_ & HS_FLAG_NEWVERSION) @@ -173,11 +173,11 @@ hstoreValidNewFormat(HStore *hs) static int hstoreValidOldFormat(HStore *hs) { - int count = hs->size_; + uint32 count = hs->size_; HOldEntry *entries = (HOldEntry *) ARRPTR(hs); - int vsize; - int lastpos = 0; - int i; + Size vsize; + Size lastpos = 0; + uint32 i; if (hs->size_ & HS_FLAG_NEWVERSION) return 0; @@ -188,7 +188,7 @@ hstoreValidOldFormat(HStore *hs) if (count > 0xFFFFFFF) return 0; - if (CALCDATASIZE(count, 0) > VARSIZE(hs)) + if (hstoreCalcDataSize(count, 0) > VARSIZE(hs)) return 0; if (entries[0].pos != 0) @@ -212,11 +212,12 @@ hstoreValidOldFormat(HStore *hs) { if (entries[i].pos != lastpos) return 0; - lastpos += (entries[i].keylen - + ((entries[i].valisnull) ? 0 : entries[i].vallen)); + lastpos = add_size(lastpos, + entries[i].keylen + + ((entries[i].valisnull) ? 0 : entries[i].vallen)); } - vsize = CALCDATASIZE(count, lastpos); + vsize = hstoreCalcDataSize(count, lastpos); if (vsize > VARSIZE(hs)) return 0; @@ -316,10 +317,10 @@ hstoreUpgrade(Datum orig) * must have an old-style value. Overwrite it in place as a new-style one. */ { - int count = hs->size_; + uint32 count = hs->size_; HEntry *new_entries = ARRPTR(hs); HOldEntry *old_entries = (HOldEntry *) ARRPTR(hs); - int i; + uint32 i; for (i = 0; i < count; ++i) { diff --git a/contrib/hstore/hstore_io.c b/contrib/hstore/hstore_io.c index e6152bbc1a3..785aa308cde 100644 --- a/contrib/hstore/hstore_io.c +++ b/contrib/hstore/hstore_io.c @@ -349,6 +349,21 @@ comparePairs(const void *a, const void *b) return (pa->keylen > pb->keylen) ? 1 : -1; } +/* + * Add the string-data length of a pair to *buflen. + * + * This is a convenience routine for add_size(), checking if a Pair is null + * before adding its size. Individual keys and values are each limited to + * HENTRY_POSMASK bytes. + */ +static void +hstoreAddPairLen(Size *buflen, const Pairs *pair) +{ + Size pairlen = pair->keylen + (pair->isnull ? 0 : pair->vallen); + + *buflen = add_size(*buflen, pairlen); +} + /* * this code still respects pairs.needfree, even though in general * it should never be called in a context where anything needs freeing. @@ -356,7 +371,7 @@ comparePairs(const void *a, const void *b) * and (b) who knows whether they might be needed by some caller. */ int -hstoreUniquePairs(Pairs *a, int32 l, int32 *buflen) +hstoreUniquePairs(Pairs *a, int32 l, Size *buflen) { Pairs *ptr, *res; @@ -365,7 +380,7 @@ hstoreUniquePairs(Pairs *a, int32 l, int32 *buflen) if (l < 2) { if (l == 1) - *buflen = a->keylen + ((a->isnull) ? 0 : a->vallen); + hstoreAddPairLen(buflen, a); return l; } @@ -391,7 +406,7 @@ hstoreUniquePairs(Pairs *a, int32 l, int32 *buflen) } else { - *buflen += res->keylen + ((res->isnull) ? 0 : res->vallen); + hstoreAddPairLen(buflen, res); res++; if (res != ptr) memcpy(res, ptr, sizeof(Pairs)); @@ -400,7 +415,7 @@ hstoreUniquePairs(Pairs *a, int32 l, int32 *buflen) ptr++; } - *buflen += res->keylen + ((res->isnull) ? 0 : res->vallen); + hstoreAddPairLen(buflen, res); return res + 1 - a; } @@ -446,16 +461,17 @@ hstoreCheckValLength(size_t len, HSParser *state) HStore * -hstorePairs(Pairs *pairs, int32 pcount, int32 buflen) +hstorePairs(Pairs *pairs, int32 pcount, Size buflen) { HStore *out; HEntry *entry; char *ptr; char *buf; - int32 len; + Size len; int32 i; - len = CALCDATASIZE(pcount, buflen); + len = hstoreCalcDataSize(pcount, buflen); + out = palloc(len); SET_VARSIZE(out, len); HS_SETCOUNT(out, pcount); @@ -482,7 +498,7 @@ hstore_in(PG_FUNCTION_ARGS) char *str = PG_GETARG_CSTRING(0); Node *escontext = fcinfo->context; HSParser state; - int32 buflen; + Size buflen; HStore *out; state.begin = str; @@ -503,7 +519,7 @@ PG_FUNCTION_INFO_V1(hstore_recv); Datum hstore_recv(PG_FUNCTION_ARGS) { - int32 buflen; + Size buflen; HStore *out; Pairs *pairs; int32 i; @@ -602,7 +618,7 @@ PG_FUNCTION_INFO_V1(hstore_from_arrays); Datum hstore_from_arrays(PG_FUNCTION_ARGS) { - int32 buflen; + Size buflen; HStore *out; Pairs *pairs; Datum *key_datums; @@ -721,7 +737,7 @@ hstore_from_array(PG_FUNCTION_ARGS) ArrayType *in_array = PG_GETARG_ARRAYTYPE_P(0); int ndims = ARR_NDIM(in_array); int count; - int32 buflen; + Size buflen; HStore *out; Pairs *pairs; Datum *in_datums; @@ -835,7 +851,7 @@ Datum hstore_from_record(PG_FUNCTION_ARGS) { HeapTupleHeader rec; - int32 buflen; + Size buflen; HStore *out; Pairs *pairs; Oid tupType; @@ -1226,8 +1242,8 @@ Datum hstore_out(PG_FUNCTION_ARGS) { HStore *in = PG_GETARG_HSTORE_P(0); - int buflen, - i; + Size buflen; + int i; int count = HS_COUNT(in); char *out, *ptr; @@ -1249,12 +1265,18 @@ hstore_out(PG_FUNCTION_ARGS) for (i = 0; i < count; i++) { - /* include "" and => and comma-space */ - buflen += 6 + 2 * HSTORE_KEYLEN(entries, i); + /* include "", => and comma-space */ + buflen = add_size(buflen, 6); + buflen = add_size(buflen, mul_size(2, HSTORE_KEYLEN(entries, i))); + /* include "" only if nonnull */ - buflen += 2 + (HSTORE_VALISNULL(entries, i) - ? 2 - : 2 * HSTORE_VALLEN(entries, i)); + if (HSTORE_VALISNULL(entries, i)) + buflen = add_size(buflen, 4); + else + { + buflen = add_size(buflen, 2); + buflen = add_size(buflen, mul_size(2, HSTORE_VALLEN(entries, i))); + } } out = ptr = palloc(buflen); diff --git a/contrib/hstore/hstore_op.c b/contrib/hstore/hstore_op.c index cab59700c0d..e461671ada4 100644 --- a/contrib/hstore/hstore_op.c +++ b/contrib/hstore/hstore_op.c @@ -76,7 +76,7 @@ hstoreArrayToPairs(ArrayType *a, int *npairs) bool *key_nulls; int key_count; Pairs *key_pairs; - int bufsiz; + Size bufsiz; int i, j; @@ -637,7 +637,7 @@ hstore_slice_to_hstore(PG_FUNCTION_ARGS) int nkeys; Pairs *key_pairs = hstoreArrayToPairs(key_array, &nkeys); Pairs *out_pairs; - int bufsiz; + Size bufsiz; int lastidx = 0; int i; int out_count = 0; @@ -1241,8 +1241,8 @@ hstore_hash(PG_FUNCTION_ARGS) */ Assert(VARSIZE(hs) == (HS_COUNT(hs) != 0 ? - CALCDATASIZE(HS_COUNT(hs), - HSE_ENDPOS(ARRPTR(hs)[2 * HS_COUNT(hs) - 1])) : + hstoreCalcDataSize(HS_COUNT(hs), + HSE_ENDPOS(ARRPTR(hs)[2 * HS_COUNT(hs) - 1])) : HSHRDSIZE)); PG_FREE_IF_COPY(hs, 0); @@ -1264,8 +1264,8 @@ hstore_hash_extended(PG_FUNCTION_ARGS) /* See comment in hstore_hash */ Assert(VARSIZE(hs) == (HS_COUNT(hs) != 0 ? - CALCDATASIZE(HS_COUNT(hs), - HSE_ENDPOS(ARRPTR(hs)[2 * HS_COUNT(hs) - 1])) : + hstoreCalcDataSize(HS_COUNT(hs), + HSE_ENDPOS(ARRPTR(hs)[2 * HS_COUNT(hs) - 1])) : HSHRDSIZE)); PG_FREE_IF_COPY(hs, 0); diff --git a/contrib/hstore/hstore_subs.c b/contrib/hstore/hstore_subs.c index 56e0858c1a6..c1695daf72f 100644 --- a/contrib/hstore/hstore_subs.c +++ b/contrib/hstore/hstore_subs.c @@ -192,7 +192,7 @@ hstore_subscript_assign(ExprState *state, HStore *hs = DatumGetHStoreP(*op->resvalue); int s1count = HS_COUNT(hs); int outcount = 0; - int vsize; + Size vsize; char *ps1, *bufd, *pd; @@ -202,7 +202,7 @@ hstore_subscript_assign(ExprState *state, int s2idx; /* Allocate result without considering possibility of duplicate */ - vsize = CALCDATASIZE(s1count + 1, VARSIZE(hs) + p.keylen + p.vallen); + vsize = hstoreCalcDataSize(s1count + 1, VARSIZE(hs) + p.keylen + p.vallen); out = palloc(vsize); SET_VARSIZE(out, vsize); HS_SETCOUNT(out, s1count + 1); diff --git a/contrib/hstore_plperl/hstore_plperl.c b/contrib/hstore_plperl/hstore_plperl.c index d7f1b8ddb48..4a6ff80dae5 100644 --- a/contrib/hstore_plperl/hstore_plperl.c +++ b/contrib/hstore_plperl/hstore_plperl.c @@ -13,9 +13,9 @@ PG_MODULE_MAGIC_EXT( /* Linkage to functions in hstore module */ typedef HStore *(*hstoreUpgrade_t) (Datum orig); static hstoreUpgrade_t hstoreUpgrade_p; -typedef int (*hstoreUniquePairs_t) (Pairs *a, int32 l, int32 *buflen); +typedef int (*hstoreUniquePairs_t) (Pairs *a, int32 l, Size *buflen); static hstoreUniquePairs_t hstoreUniquePairs_p; -typedef HStore *(*hstorePairs_t) (Pairs *pairs, int32 pcount, int32 buflen); +typedef HStore *(*hstorePairs_t) (Pairs *pairs, int32 pcount, Size buflen); static hstorePairs_t hstorePairs_p; typedef size_t (*hstoreCheckKeyLen_t) (size_t len); static hstoreCheckKeyLen_t hstoreCheckKeyLen_p; @@ -104,7 +104,7 @@ plperl_to_hstore(PG_FUNCTION_ARGS) SV *in = (SV *) PG_GETARG_POINTER(0); HV *hv; HE *he; - int32 buflen; + Size buflen; int32 i; int32 pcount; HStore *out; diff --git a/contrib/hstore_plpython/hstore_plpython.c b/contrib/hstore_plpython/hstore_plpython.c index b9d8b4537f7..6e726805dfd 100644 --- a/contrib/hstore_plpython/hstore_plpython.c +++ b/contrib/hstore_plpython/hstore_plpython.c @@ -19,9 +19,9 @@ static PLyUnicode_FromStringAndSize_t PLyUnicode_FromStringAndSize_p; /* Linkage to functions in hstore module */ typedef HStore *(*hstoreUpgrade_t) (Datum orig); static hstoreUpgrade_t hstoreUpgrade_p; -typedef int (*hstoreUniquePairs_t) (Pairs *a, int32 l, int32 *buflen); +typedef int (*hstoreUniquePairs_t) (Pairs *a, int32 l, Size *buflen); static hstoreUniquePairs_t hstoreUniquePairs_p; -typedef HStore *(*hstorePairs_t) (Pairs *pairs, int32 pcount, int32 buflen); +typedef HStore *(*hstorePairs_t) (Pairs *pairs, int32 pcount, Size buflen); static hstorePairs_t hstorePairs_p; typedef size_t (*hstoreCheckKeyLen_t) (size_t len); static hstoreCheckKeyLen_t hstoreCheckKeyLen_p; @@ -156,7 +156,7 @@ plpython_to_hstore(PG_FUNCTION_ARGS) PG_TRY(); { - int32 buflen; + Size buflen; Py_ssize_t i; Pairs *pairs;