agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedFrom: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Ewan Young <kdbase.hack@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Prevent crash when calling pgstat functions with unregistered stats kind
Date: Thu, 2 Jul 2026 04:43:32 +0000
Message-ID: <akXsdHjBMbQk9qNT@bdtpg> (raw)
In-Reply-To: <akXntNv2344hoc6L@bdtpg>
References: <akS/ldidWeqG1FWk@bdtpg>
<CAON2xHOA23Oq8EERoV9ERBZof1O_vN-tYf0q58TwdkrTNZ-SPg@mail.gmail.com>
<akXalpb3zjPX3ZEl@bdtpg>
<akXebziFr_eQgQi8@paquier.xyz>
<akXjqbXRTLNcwHyE@bdtpg>
<akXkqov6wLbKwpAd@paquier.xyz>
<akXntNv2344hoc6L@bdtpg>
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
Attachments:
[text/x-diff] v3-0001-Fix-test_custom_stats-modules-to-error-out-when-n.patch (2.7K, ../akXsdHjBMbQk9qNT@bdtpg/2-v3-0001-Fix-test_custom_stats-modules-to-error-out-when-n.patch)
download | inline diff:
From 7468051aa68c0571e2502bcfee3e2a319e96069e Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Date: Thu, 2 Jul 2026 04:34:49 +0000
Subject: [PATCH v3] Fix test_custom_stats modules to error out when not
preloaded
Previously, test_custom_var_stats and test_custom_fixed_stats silently
skipped pgstat_register_kind() when not loaded via shared_preload_libraries.
This left the SQL functions callable without the kind registered, leading to NULL
dereferences in core pgstat functions.
Remove the early return and let pgstat_register_kind() error out naturally,
matching the pattern used by test_custom_rmgrs with RegisterCustomRmgr(). Now
CREATE EXTENSION fails with a clear error if the module is not preloaded.
Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Reviewed-by: Ewan Young <kdbase.hack@gmail.com>
Discussion: https://postgr.es/m/akS/ldidWeqG1FWk%40bdtpg
---
.../modules/test_custom_stats/test_custom_fixed_stats.c | 9 ++++-----
.../modules/test_custom_stats/test_custom_var_stats.c | 9 ++++-----
2 files changed, 8 insertions(+), 10 deletions(-)
100.0% src/test/modules/test_custom_stats/
diff --git a/src/test/modules/test_custom_stats/test_custom_fixed_stats.c b/src/test/modules/test_custom_stats/test_custom_fixed_stats.c
index a066ce117a6..8b29ca3e27a 100644
--- a/src/test/modules/test_custom_stats/test_custom_fixed_stats.c
+++ b/src/test/modules/test_custom_stats/test_custom_fixed_stats.c
@@ -72,11 +72,10 @@ static const PgStat_KindInfo custom_stats = {
void
_PG_init(void)
{
- /* Must be loaded via shared_preload_libraries */
- if (!process_shared_preload_libraries_in_progress)
- return;
-
- /* Register custom statistics kind */
+ /*
+ * In order to register our custom statistics kind, we have to be loaded
+ * via shared_preload_libraries. Otherwise, registration will fail.
+ */
pgstat_register_kind(PGSTAT_KIND_TEST_CUSTOM_FIXED_STATS, &custom_stats);
}
diff --git a/src/test/modules/test_custom_stats/test_custom_var_stats.c b/src/test/modules/test_custom_stats/test_custom_var_stats.c
index 863d6a52492..fa415c81301 100644
--- a/src/test/modules/test_custom_stats/test_custom_var_stats.c
+++ b/src/test/modules/test_custom_stats/test_custom_var_stats.c
@@ -129,11 +129,10 @@ static const PgStat_KindInfo custom_stats = {
void
_PG_init(void)
{
- /* Must be loaded via shared_preload_libraries */
- if (!process_shared_preload_libraries_in_progress)
- return;
-
- /* Register custom statistics kind */
+ /*
+ * In order to register our custom statistics kind, we have to be loaded
+ * via shared_preload_libraries. Otherwise, registration will fail.
+ */
pgstat_register_kind(PGSTAT_KIND_TEST_CUSTOM_VAR_STATS, &custom_stats);
}
--
2.34.1
view thread (13+ messages) latest in thread
Message-ID: <akXsdHjBMbQk9qNT@bdtpg>
Permalink: ../akXsdHjBMbQk9qNT@bdtpg/
Also on: postgresql.org/message-id/akXsdHjBMbQk9qNT@bdtpg
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: bertranddrouvot.pg@gmail.com, michael@paquier.xyz, kdbase.hack@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: Prevent crash when calling pgstat functions with unregistered stats kind
In-Reply-To: <akXsdHjBMbQk9qNT@bdtpg>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox