From 64ba49861b81dd1a71aa0ff1817c737eb441cbe3 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Mon, 17 Aug 2026 11:39:11 +0900 Subject: [PATCH v3] hstore: Rework module to use Size and {add,mul}_size for overflows --- contrib/hstore/hstore.h | 35 ++++++++++------ contrib/hstore/hstore_compat.c | 29 ++++++------- contrib/hstore/hstore_io.c | 51 +++++++++++++++-------- 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, 84 insertions(+), 59 deletions(-) diff --git a/contrib/hstore/hstore.h b/contrib/hstore/hstore.h index 897af244a423..39cbe4d1010c 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 3a9f7f45cb71..1e2db807edf7 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 9b72efb8674a..6fc29a211b25 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; @@ -1250,11 +1266,10 @@ hstore_out(PG_FUNCTION_ARGS) for (i = 0; i < count; i++) { /* include "" and => and comma-space */ - buflen += 6 + 2 * HSTORE_KEYLEN(entries, i); + buflen = add_size(buflen, add_size(6, mul_size(2, HSTORE_KEYLEN(entries, i)))); /* include "" only if nonnull */ - buflen += 2 + (HSTORE_VALISNULL(entries, i) - ? 2 - : 2 * HSTORE_VALLEN(entries, i)); + buflen = add_size(buflen, HSTORE_VALISNULL(entries, i) ? 4 : + add_size(2, 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 bcba75f92580..abc65ea62ff4 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 56e0858c1a67..c1695daf72fe 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 d7f1b8ddb488..4a6ff80dae54 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 b9d8b4537f78..6e726805dfd8 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; -- 2.55.0