| From: | Grigorev Jurij <ju(dot)grigorev(at)ftdata(dot)ru> |
|---|---|
| To: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Cc: | "heikki(dot)linnakangas(at)enterprisedb(dot)com" <heikki(dot)linnakangas(at)enterprisedb(dot)com> |
| Subject: | [PATCH v1] Reject zero resource kinds in test_resowner_many() |
| Date: | 2026-09-11 08:51:30 |
| Message-ID: | 1905e9d709554733b418d06a4806436f@localhost.localdomain |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi hackers,
While running Clang Static Analyzer through CodeChecker over PostgreSQL
18.6, we found a possible division by zero in
RememberManyTestResources():
kind_idx = (kind_idx + 1) % nkinds;
The SQL entry point rejects negative values of nkinds but accepts zero.
If there are resources to remember, the helper then accesses kinds[0]
even though the array contains no resource kinds, and can eventually
reach the modulo operation.
For example:
CREATE EXTENSION test_resowner;
SELECT test_resowner_many(0, 1, 0, 0, 0);
On an assertion-enabled master build on macOS, I get:
NOTICE: remembering 1 before-locks resources
TRAP: failed Assert("kind->release_phase != 0"),
File: "../src/backend/utils/resowner/resowner.c", Line: 536
LOG: client backend was terminated by signal 6: Abort trap: 6
ResourceOwnerRemember() detects the invalid descriptor passed as
&kinds[0].desc, so this build fails before reaching the division.
The after-locks path has the same problem:
SELECT test_resowner_many(0, 0, 0, 1, 0);
Interestingly, in the same build this failed the neighboring assertion:
TRAP: failed Assert("kind->release_priority != 0"),
File: "../src/backend/utils/resowner/resowner.c", Line: 537
The two calls fail at different checks depending on the contents of the
invalid kinds[0] entry. Without assertions, the code has already
invoked undefined behavior before reaching the modulo, so the exact
failure mode is not predictable.
ForgetManyTestResources() contains another "% nkinds", but that
expression is inside a loop bounded by nkinds. With nkinds equal to
zero, the loop body is never entered, so only the remember path needs
fixing.
The patch rejects nkinds <= 0 at the SQL entry point, matching
test_resowner_priorities() in the same module. It also adds an
assertion documenting the helper's precondition and regression coverage
for both remember paths.
This deliberately changes the behavior of the all-zero invocation:
SELECT test_resowner_many(0, 0, 0, 0, 0);
It currently succeeds as a no-op, but with the patch it returns:
ERROR: nkinds must be greater than zero
I think rejecting it is preferable because zero resource kinds do not
have a useful meaning for this test. Would it be better to preserve
the all-zero no-op case and reject nkinds = 0 only when resources are
requested?
The issue was introduced with the test_resowner module in commit
b8bff07daa85, so PostgreSQL 17, PostgreSQL 18, and master are affected.
It is confined to the test module; production ResourceOwner code is not
affected.
The patch applies cleanly to current master. I reproduced both failures
without the patch and verified that the test_resowner regression test
passes with it applied.
I would also appreciate opinions on whether this test-only fix is worth
back-patching to PostgreSQL 17 and 18. I can prepare separate
back-branch patches if needed.
I will add the patch to the CommitFest.
Best regards,
Yuriy Grigoryev
| Attachment | Content-Type | Size |
|---|---|---|
| v1-0001-Reject-zero-resource-kinds-in-test_resowner_many.patch | application/octet-stream | 3.2 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Amit Langote | 2026-09-11 08:52:27 | Re: Revert RI fast-path batching from REL_19_STABLE |
| Previous Message | Fujii Masao | 2026-09-11 08:47:22 | Re: Fix failing assert in deferred constraint trigger |