Re: [PATCH v1] Reject zero resource kinds in test_resowner_many()

On Fri, Oct 9, 2026 at 12:32 AM Grigorev Jurij <ju(dot)grigorev(at)ftdata(dot)ru>
wrote:

> 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

Hi Yuriy,

I reviewed v1 on master with assertions enabled. The test_resowner
regression test passes. Both zero-kind remember cases abort without the
patch and return the expected SQL error with it. I found no blocking issues
with this test-code fix.

Kind regards,
Sho Ito

--
＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝
*Sho Ito (Sean)*

E-mail: i(dot)sho(dot)0628(at)gmail(dot)com
LinkedIn: *https://www.linkedin.com/in/sho-ito-0628/
<https://www.linkedin.com/in/sho-ito-0628/>*
GitHub: https://github.com/sean0628/
＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝＝