Re: [BUG] hstore integer overflow when constructing large values

Michael Paquier <michael(at)paquier(dot)xyz> 于2026年8月16日周日 14:45写道：
>
> 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.

Thanks for the review.

The reason I tried to keep `int32` for `buflen` in v1 was that
`hstoreUniquePairs()` and `hstorePairs()` are exported functions. I was
trying to keep the change minimally invasive in case the fix needed to be
backpatched, and therefore avoided changing their argument types.

Given that this does not seem worth backpatching, I agree that there is no
good reason to preserve the existing `int32`-based size calculations.
I've reworked the patch to use `Size` throughout the allocation-size
calculation instead.

In v2:

* `buflen` and the final allocation length are changed to `Size`, including
the callers of `hstoreUniquePairs()` and `hstorePairs()`.

* The accumulation of key/value lengths uses `add_size()` instead of
unchecked integer arithmetic.

* `CALCDATASIZE()` now uses checked `Size` arithmetic with `add_size()` and
`mul_size()`, rather than relying on assumptions about the maximum values
of its arguments.

* Other size calculations on the same paths, including `HS_FINALIZE()`,
`HS_FIXSIZE()`, and the output buffer calculation in `hstore_out()`, have
also been adjusted to avoid using `int`/`int32` for allocation sizes.

With these changes, the original reproducer now reports:

ERROR: invalid memory alloc request size 2160000035

rather than:

ERROR: invalid memory alloc request size 18446744071574584355

The former is the actual size of the hstore representation being passed to
`palloc()`: 2160000003 bytes of key/value data plus 32 bytes for the hstore
header and HEntry array. Thus the size calculation no longer overflows an
`int32`, and the allocation limit is handled by the normal palloc machinery.

Attached is v2.

--
Thanks,
Tender Wang