Re: Post-release followup: pg_add_size_overflow() - Mailing list pgsql-hackers

From Chao Li
Subject Re: Post-release followup: pg_add_size_overflow()
Date
Msg-id 7111A2BE-6AE8-49DD-B67E-C2E42403038A@gmail.com
Whole thread Raw
In response to Post-release followup: pg_add_size_overflow()  (Jacob Champion <jacob.champion@enterprisedb.com>)
List pgsql-hackers
Hi, Jacob,

I just reviewed the patch. Overall looks solid to me. Just a small comments:

> On Nov 19, 2025, at 04:09, Jacob Champion <jacob.champion@enterprisedb.com> wrote:
>
> Hi all,
>
> 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.
>
> 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.
>
> 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.
>
> 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.)
>
> 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.
>
> Thanks!
> --Jacob
> <0001-Add-pg_add_size_overflow-and-friends.patch><0002-postgres-Use-pg_-add-mul-_size_overflow.patch>

1 - 0001
```
+/*
+ * pg_neg_size_overflow is currently omitted, to avoid having to reason about
+ * the portability of SSIZE_MIN/_MAX before a use case exists.
+ */
+#if 0
+static inline bool
+pg_neg_size_overflow(size_t a, ssize_t *result)
+{
+    ...
+}
+#endif
```

Putting “…” inside a function body looks quite uncommon. I searched over the source tree and couldn't find other
occurrence.As the comment has explained why omitting pg_neg_size_overflow, maybe just remove the entry #if 0 block, or
justleave an empty function body. 

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/







pgsql-hackers by date:

Previous
From: Peter Geoghegan
Date:
Subject: Re: Add a berief general comment on BTScanInsertData's nextkey and backward
Next
From: Andrew Kim
Date:
Subject: Re: Proposal for enabling auto-vectorization for checksum calculations