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
signature.asc
Description: PGP signature
