Re: Post-release followup: pg_add_size_overflow()

From: Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>
To: Jacob Champion <jacob(dot)champion(at)enterprisedb(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Post-release followup: pg_add_size_overflow()
Date: 2025-11-19 01:17:07
Message-ID: 7111A2BE-6AE8-49DD-B67E-C2E42403038A@gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: 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(dot)champion(at)enterprisedb(dot)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 just leave an empty function body.

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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Kim 2025-11-19 01:18:36 Re: Proposal for enabling auto-vectorization for checksum calculations
Previous Message Peter Geoghegan 2025-11-19 01:15:40 Re: Add a berief general comment on BTScanInsertData's nextkey and backward