Prevent crash when calling pgstat functions with unregistered stats kind

Hi Bertrand,

On Wed, Jul 1, 2026 at 3:20 PM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:

Hi hackers,

While reviewing [1], I got segfault(s) because I created a custom statistics
extension that I forgot to add to shared_preload_libraries. Then using one of
its function produced:

"
Core was generated by `postgres: postgres postgres [local] SELECT '.
Program terminated with signal SIGSEGV, Segmentation fault.
#0 pgstat_init_entry (kind=kind@entry=24, shhashent=shhashent@entry=0x73f6c341a740) at pgstat_shmem.c:335
335 chunk = dsa_allocate_extended(pgStatLocal.dsa,
"

Indeed, if a custom statistics extension is loaded via CREATE EXTENSION without
being listed in shared_preload_libraries, its _PG_init() skips the call to
pgstat_register_kind(). The SQL functions are still created, and calling them
invokes pgstat functions with a kind that was never registered.

pgstat_get_kind_info() returns NULL in this case. The existing code only
checked this via Assert() in some paths, so non-assert builds would dereference
NULL and segfault.

The attached patch adds runtime checks in all public-facing pgstat functions that
accept a PgStat_Kind and dereference the returned kind info:

- pgstat_prep_pending_entry()
- pgstat_fetch_entry()
- pgstat_reset()
- pgstat_reset_of_kind()
- pgstat_have_entry()
- pgstat_snapshot_fixed()
- pgstat_init_entry()
- pgstat_reset_entry()

Each now raises ERROR with ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE when
the kind is not known or registered.

Thanks for the patch — nice catch, and the diagnosis looks right.

One small thing: in pgstat_snapshot_fixed(), the existing
Assert(pgstat_is_kind_valid(kind)); becomes redundant after the new NULL
check. A non-NULL kind_info already implies the kind is valid (that's the
only way pgstat_get_kind_info() returns non-NULL), so the assert can never
fire. Might as well drop it and keep just the fixed_amount one.

[1]: /messages/by-id/akSi2txzLZWQL31Q@bdtpg

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

--
Regards,
Ewan Young

Hi Ewan,

On Wed, Jul 01, 2026 at 04:20:39PM +0800, Ewan Young wrote:

Thanks for the patch — nice catch, and the diagnosis looks right.

Thanks for looking at it!

One small thing: in pgstat_snapshot_fixed(), the existing
Assert(pgstat_is_kind_valid(kind)); becomes redundant after the new NULL
check. A non-NULL kind_info already implies the kind is valid (that's the
only way pgstat_get_kind_info() returns non-NULL), so the assert can never
fire. Might as well drop it and keep just the fixed_amount one.

Yeah good catch, done in the attached.

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

On Thu, Jul 02, 2026 at 03:27:18AM +0000, Bertrand Drouvot wrote:

On Wed, Jul 01, 2026 at 04:20:39PM +0800, Ewan Young wrote:

One small thing: in pgstat_snapshot_fixed(), the existing
Assert(pgstat_is_kind_valid(kind)); becomes redundant after the new NULL
check. A non-NULL kind_info already implies the kind is valid (that's the
only way pgstat_get_kind_info() returns non-NULL), so the assert can never
fire. Might as well drop it and keep just the fixed_amount one.

Yeah good catch, done in the attached.

I am not convinced that it is worth bothering in the core code about
this class of failures; they are just not interesting, and impossible
to miss.

It seems to me that this error is in the _PG_init() of the modules in
modules/test_custom_stats/: we should not bypass the
pgstat_register_kind() if not loading the library from
shared_preload_libraries, but let the call happen and fail.
--
Michael

Hi,

On Thu, Jul 02, 2026 at 12:43:43PM +0900, Michael Paquier wrote:

On Thu, Jul 02, 2026 at 03:27:18AM +0000, Bertrand Drouvot wrote:

On Wed, Jul 01, 2026 at 04:20:39PM +0800, Ewan Young wrote:

One small thing: in pgstat_snapshot_fixed(), the existing
Assert(pgstat_is_kind_valid(kind)); becomes redundant after the new NULL
check. A non-NULL kind_info already implies the kind is valid (that's the
only way pgstat_get_kind_info() returns non-NULL), so the assert can never
fire. Might as well drop it and keep just the fixed_amount one.

Yeah good catch, done in the attached.

I am not convinced that it is worth bothering in the core code about
this class of failures; they are just not interesting, and impossible
to miss.

It seems to me that this error is in the _PG_init() of the modules in
modules/test_custom_stats/: we should not bypass the
pgstat_register_kind() if not loading the library from
shared_preload_libraries, but let the call happen and fail.

I agree that the responsibility should primarily be in the extension. However,
the issue is that the NULL dereference happens inside core code (pgstat_prep_pending_entry,
etc.), and the resulting segfault(s) cause the postmaster to terminate all
backends (not just the offending session).

Given that one misconfigured extension can crash all connections on the server,
a defensive check in core seems reasonable (kind of similar to 341e9a05e7b).

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

On Thu, Jul 02, 2026 at 04:06:01AM +0000, Bertrand Drouvot wrote:

I agree that the responsibility should primarily be in the extension. However,
the issue is that the NULL dereference happens inside core code (pgstat_prep_pending_entry,
etc.), and the resulting segfault(s) cause the postmaster to terminate all
backends (not just the offending session).

Given that one misconfigured extension can crash all connections on the server,
a defensive check in core seems reasonable (kind of similar to 341e9a05e7b).

Nope, this was a different thing, doable in a couple of steps:
- Load the library.
- Write custom stats.
- Stop the server, flush the stats.
- Edit the configuration, not loading the library.
- Restart the server, loading failed.

The problem of this thread ought to be blocked at its source, in the
extension itself: let's not give free hands to an extension to do what
it should not be allowed to do. There is a similar defense in
test_custom_rmgrs, as one example. We should just map to that.
--
Michael

Hi,

On Thu, Jul 02, 2026 at 01:10:18PM +0900, Michael Paquier wrote:

On Thu, Jul 02, 2026 at 04:06:01AM +0000, Bertrand Drouvot wrote:

I agree that the responsibility should primarily be in the extension. However,
the issue is that the NULL dereference happens inside core code (pgstat_prep_pending_entry,
etc.), and the resulting segfault(s) cause the postmaster to terminate all
backends (not just the offending session).

Given that one misconfigured extension can crash all connections on the server,
a defensive check in core seems reasonable (kind of similar to 341e9a05e7b).

Nope, this was a different thing, doable in a couple of steps:
- Load the library.
- Write custom stats.
- Stop the server, flush the stats.
- Edit the configuration, not loading the library.
- Restart the server, loading failed.

The problem of this thread ought to be blocked at its source, in the
extension itself: let's not give free hands to an extension to do what
it should not be allowed to do. There is a similar defense in
test_custom_rmgrs, as one example. We should just map to that.

Ok but what about extensions that don't call pgstat_register_kind() at all? Your
point is that they would see the issue during the development of the extension? (If
so, I think I could agree).

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

On Thu, Jul 02, 2026 at 04:23:16AM +0000, Bertrand Drouvot wrote:

Hi,

On Thu, Jul 02, 2026 at 01:10:18PM +0900, Michael Paquier wrote:

On Thu, Jul 02, 2026 at 04:06:01AM +0000, Bertrand Drouvot wrote:

I agree that the responsibility should primarily be in the extension. However,
the issue is that the NULL dereference happens inside core code (pgstat_prep_pending_entry,
etc.), and the resulting segfault(s) cause the postmaster to terminate all
backends (not just the offending session).

Given that one misconfigured extension can crash all connections on the server,
a defensive check in core seems reasonable (kind of similar to 341e9a05e7b).

Nope, this was a different thing, doable in a couple of steps:
- Load the library.
- Write custom stats.
- Stop the server, flush the stats.
- Edit the configuration, not loading the library.
- Restart the server, loading failed.

The problem of this thread ought to be blocked at its source, in the
extension itself: let's not give free hands to an extension to do what
it should not be allowed to do. There is a similar defense in
test_custom_rmgrs, as one example. We should just map to that.

Ok but what about extensions that don't call pgstat_register_kind() at all? Your
point is that they would see the issue during the development of the extension? (If
so, I think I could agree).

Something like in the attached?

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

On Thu, Jul 02, 2026 at 04:43:32AM +0000, Bertrand Drouvot wrote:

On Thu, Jul 02, 2026 at 04:23:16AM +0000, Bertrand Drouvot wrote:

Ok but what about extensions that don't call pgstat_register_kind() at all? Your
point is that they would see the issue during the development of the extension? (If
so, I think I could agree).

Extensions doing custom stats have to call the register API, or
they're broken. This is the same assumption as custom RMGRs. There
are many ways to break the backend if you don't know what you do, just
take hooks for example. That's just one of them.

Something like in the attached?

Yes, that looks OK here.
--
Michael

Hi,

On Thu, Jul 02, 2026 at 02:00:06PM +0900, Michael Paquier wrote:

On Thu, Jul 02, 2026 at 04:43:32AM +0000, Bertrand Drouvot wrote:

On Thu, Jul 02, 2026 at 04:23:16AM +0000, Bertrand Drouvot wrote:

Ok but what about extensions that don't call pgstat_register_kind() at all? Your
point is that they would see the issue during the development of the extension? (If
so, I think I could agree).

Extensions doing custom stats have to call the register API, or
they're broken. This is the same assumption as custom RMGRs. There
are many ways to break the backend if you don't know what you do, just
take hooks for example. That's just one of them.

Something like in the attached?

Yes, that looks OK here.

That makes sense, I do agree.

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

On Thu, Jul 02, 2026 at 05:02:36AM +0000, Bertrand Drouvot wrote:

That makes sense, I do agree.

Done that now down to v19, thanks, in the shape of some pure code
deletion.
--
Michael