Re: Post-release followup: pg_add_size_overflow()

On Tue, Nov 18, 2025 at 12:09:17PM -0800, Jacob Champion wrote:
> The fix for CVE-2025-12818 introduced a few identical copies of size_t
> addition, and now that we've released, I'd like to pull those back
> into shape.

Yes, I've noticed these TODOs in the final patch.  Thanks for the
follow-up cleanup.

> 0001 replaces the bespoke code with a new size_t implementation of the
> operators in common/int.h. 0002 additionally makes use of these in
> shmem.c, because I couldn't think of a good reason not to.

Sounds good to me.

> Couple things to note:
> 
> 1) The backend's add_size(), which I patterned the CVE fix on
> originally, checks if the result is less than either operand. The
> common/int.h implementations check only the *first* operand, which
> also looks correct to me -- if (result < a), it must also be true that
> (result < b), because otherwise (result - b) is nonnegative and we
> couldn't have overflowed the addition in the first place. But my brain
> is a little fried from looking at these problems, and I could use a +1
> from someone with fresh eyes.

This is following the same pattern as what has been introduced in
7dedfd22b798 for the other unsigned types in int.h.  Anyway, looking
at that separately, the logic of 0001 seems right here.

> 2) I have not implemented pg_neg_size_overflow(), because to me it
> seems likely to be permanently dead code, and it would require
> additional reasoning about the portability of SSIZE_MAX.
> (pg_sub_size_overflow(), by contrast, is easy to do and feels like it
> might be useful to someone eventually.)

Documenting the portability issue is important, indeed.  I'd suggest
to not use a ifdef 0, though, which may be confusing on grep if one
does not look at the surrounding lines.  Leaving that in the shape of
a comment would be hard to miss.  Anyway, are you worrying about
SIZE_MAX matching with something different than the compile-time value
at runtime?  If we don't have use for a neg subroutine yet, leaving
that out of the picture for now is fine here.  

> I don't currently plan to backport this, because I don't think the
> delta is likely to cause anyone additional pain in the future, but let
> me know if you disagree.

Keeping this cleanup on HEAD sounds fine to me.
--
Michael