public inbox for [email protected]
help / color / mirror / Atom feed[PATCH] Fix NULL dereference in pg_get_database_ddl()
4+ messages / 2 participants
[nested] [flat]
* [PATCH] Fix NULL dereference in pg_get_database_ddl()
@ 2026-04-10 13:57 Ayush Tiwari <[email protected]>
2026-04-10 14:13 ` Re: [PATCH] Fix NULL dereference in pg_get_database_ddl() Ayush Tiwari <[email protected]>
2026-04-13 09:32 ` Re: [PATCH] Fix NULL dereference in pg_get_database_ddl() David Rowley <[email protected]>
0 siblings, 2 replies; 4+ messages in thread
From: Ayush Tiwari @ 2026-04-10 13:57 UTC (permalink / raw)
To: pgsql-hackers
Hi,
pg_get_database_ddl_internal() can dereference a NULL pointer when
pg_database.dattablespace points to a tablespace OID that no longer
exists.
The immediate issue is that get_tablespace_name() may return NULL, but
the result is passed directly to pg_strcasecmp():
spcname = get_tablespace_name(dbform->dattablespace);
if (pg_strcasecmp(spcname, "pg_default") != 0)
...
That leads to a backend crash. I reproduced it on current master as a
SIGSEGV with crash recovery.
This function was introduced by commit a4f774cf1c7.
Deterministic reproduction:
CREATE DATABASE regression_testdb;
SET allow_system_table_mods = on;
UPDATE pg_database
SET dattablespace = 99999
WHERE datname = 'regression_testdb';
RESET allow_system_table_mods;
SELECT * FROM pg_get_database_ddl('regression_testdb');
The attached patch fixes this by checking for NULL before calling
pg_strcasecmp(). In that case, pg_get_database_ddl() simply omits the
TABLESPACE clause.
I also added a regression test in database_ddl.sql that exercises this
case by setting dattablespace to a non-existent OID and verifying that
the function returns successfully.
Patch attached. Please review and let me know if it needs any edits. Thanks!
Regards,
Ayush Tiwari
Attachments:
[application/octet-stream] 0001-Fix-NULL-dereference-in-pg_get_database_ddl.patch (4.7K, 3-0001-Fix-NULL-dereference-in-pg_get_database_ddl.patch)
download | inline diff:
From ed94357fbd47b9c29434e92b9ee65b4bb8d3f6f1 Mon Sep 17 00:00:00 2001
From: Ayush Tiwari <[email protected]>
Date: Fri, 10 Apr 2026 19:17:12 +0530
Subject: [PATCH] Fix NULL dereference in pg_get_database_ddl()
pg_get_database_ddl_internal() can dereference a NULL pointer when
pg_database.dattablespace points to a tablespace OID that no longer
exists.
The immediate issue is that get_tablespace_name() may return NULL, but
the result is passed directly to pg_strcasecmp():
spcname = get_tablespace_name(dbform->dattablespace);
if (pg_strcasecmp(spcname, "pg_default") != 0)
...
That leads to a backend crash (SIGSEGV).
This function was introduced by commit a4f774cf1c7.
The patch fixes this by checking for NULL before calling
pg_strcasecmp(). In that case, pg_get_database_ddl() simply omits the
TABLESPACE clause.
A regression test is added in database_ddl.sql that exercises this
case by setting dattablespace to a non-existent OID and verifying that
the function returns successfully without crashing.
---
src/backend/utils/adt/ddlutils.c | 2 +-
src/test/regress/expected/database_ddl.out | 24 ++++++++++++++++++++++
src/test/regress/sql/database_ddl.sql | 17 +++++++++++++++
3 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/src/backend/utils/adt/ddlutils.c b/src/backend/utils/adt/ddlutils.c
index b16c277d000..c5885b18958 100644
--- a/src/backend/utils/adt/ddlutils.c
+++ b/src/backend/utils/adt/ddlutils.c
@@ -976,7 +976,7 @@ pg_get_database_ddl_internal(Oid dbid, bool pretty,
{
char *spcname = get_tablespace_name(dbform->dattablespace);
- if (pg_strcasecmp(spcname, "pg_default") != 0)
+ if (spcname != NULL && pg_strcasecmp(spcname, "pg_default") != 0)
append_ddl_option(&buf, pretty, 4, "TABLESPACE = %s",
quote_identifier(spcname));
}
diff --git a/src/test/regress/expected/database_ddl.out b/src/test/regress/expected/database_ddl.out
index 97657e52cfa..bfd7cdcbacf 100644
--- a/src/test/regress/expected/database_ddl.out
+++ b/src/test/regress/expected/database_ddl.out
@@ -83,6 +83,30 @@ ERROR: permission denied for database regression_database_ddl
RESET ROLE;
GRANT CONNECT ON DATABASE regression_database_ddl TO PUBLIC;
DROP ROLE regress_db_ddl_noaccess;
+-- Test for dropped tablespace: dattablespace pointing to a non-existent OID
+-- should not crash (see commit fixing NULL deref in pg_get_database_ddl_internal)
+CREATE DATABASE regression_database_ddl2
+ ENCODING utf8 LC_COLLATE "C" LC_CTYPE "C" TEMPLATE template0
+ OWNER regress_datdba;
+SET allow_system_table_mods = on;
+UPDATE pg_database SET dattablespace = 99999
+ WHERE datname = 'regression_database_ddl2';
+UPDATE 1
+RESET allow_system_table_mods;
+SELECT ddl_filter(pg_get_database_ddl) FROM pg_get_database_ddl('regression_database_ddl2');
+ ddl_filter
+------------------------------------------------------------------------------------------
+ CREATE DATABASE regression_database_ddl2 WITH TEMPLATE = template0 ENCODING = 'UTF8';
+ ALTER DATABASE regression_database_ddl2 OWNER TO regress_datdba;
+(2 rows)
+
+SET allow_system_table_mods = on;
+UPDATE pg_database SET dattablespace =
+ (SELECT oid FROM pg_tablespace WHERE spcname = 'pg_default')
+ WHERE datname = 'regression_database_ddl2';
+UPDATE 1
+RESET allow_system_table_mods;
+DROP DATABASE regression_database_ddl2;
DROP DATABASE regression_database_ddl;
DROP FUNCTION ddl_filter(text);
DROP ROLE regress_datdba;
diff --git a/src/test/regress/sql/database_ddl.sql b/src/test/regress/sql/database_ddl.sql
index 89753ac6411..0792a4a9f35 100644
--- a/src/test/regress/sql/database_ddl.sql
+++ b/src/test/regress/sql/database_ddl.sql
@@ -61,6 +61,23 @@ RESET ROLE;
GRANT CONNECT ON DATABASE regression_database_ddl TO PUBLIC;
DROP ROLE regress_db_ddl_noaccess;
+-- Test for dropped tablespace: dattablespace pointing to a non-existent OID
+-- should not crash (see commit fixing NULL deref in pg_get_database_ddl_internal)
+CREATE DATABASE regression_database_ddl2
+ ENCODING utf8 LC_COLLATE "C" LC_CTYPE "C" TEMPLATE template0
+ OWNER regress_datdba;
+SET allow_system_table_mods = on;
+UPDATE pg_database SET dattablespace = 99999
+ WHERE datname = 'regression_database_ddl2';
+RESET allow_system_table_mods;
+SELECT ddl_filter(pg_get_database_ddl) FROM pg_get_database_ddl('regression_database_ddl2');
+SET allow_system_table_mods = on;
+UPDATE pg_database SET dattablespace =
+ (SELECT oid FROM pg_tablespace WHERE spcname = 'pg_default')
+ WHERE datname = 'regression_database_ddl2';
+RESET allow_system_table_mods;
+DROP DATABASE regression_database_ddl2;
+
DROP DATABASE regression_database_ddl;
DROP FUNCTION ddl_filter(text);
DROP ROLE regress_datdba;
--
2.53.0.windows.2
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: [PATCH] Fix NULL dereference in pg_get_database_ddl()
2026-04-10 13:57 [PATCH] Fix NULL dereference in pg_get_database_ddl() Ayush Tiwari <[email protected]>
@ 2026-04-10 14:13 ` Ayush Tiwari <[email protected]>
1 sibling, 0 replies; 4+ messages in thread
From: Ayush Tiwari @ 2026-04-10 14:13 UTC (permalink / raw)
To: pgsql-hackers
On Fri, 10 Apr 2026 at 19:27, Ayush Tiwari <[email protected]>
wrote:
> Hi,
>
> pg_get_database_ddl_internal() can dereference a NULL pointer when
> pg_database.dattablespace points to a tablespace OID that no longer
> exists.
>
> The immediate issue is that get_tablespace_name() may return NULL, but
> the result is passed directly to pg_strcasecmp():
>
> spcname = get_tablespace_name(dbform->dattablespace);
> if (pg_strcasecmp(spcname, "pg_default") != 0)
> ...
>
> That leads to a backend crash. I reproduced it on current master as a
> SIGSEGV with crash recovery.
>
> This function was introduced by commit a4f774cf1c7.
>
> Deterministic reproduction:
>
> CREATE DATABASE regression_testdb;
> SET allow_system_table_mods = on;
> UPDATE pg_database
> SET dattablespace = 99999
> WHERE datname = 'regression_testdb';
> RESET allow_system_table_mods;
>
> SELECT * FROM pg_get_database_ddl('regression_testdb');
>
> The attached patch fixes this by checking for NULL before calling
> pg_strcasecmp(). In that case, pg_get_database_ddl() simply omits the
> TABLESPACE clause.
>
> I also added a regression test in database_ddl.sql that exercises this
> case by setting dattablespace to a non-existent OID and verifying that
> the function returns successfully.
>
> Patch attached. Please review and let me know if it needs any edits.
> Thanks!
>
> Regards,
> Ayush Tiwari
>
Re-attaching patch without trailing white-space
Regards,
Ayush
Attachments:
[application/octet-stream] 0001-Fix-NULL-dereference-in-pg_get_database_ddl.patch (4.7K, 3-0001-Fix-NULL-dereference-in-pg_get_database_ddl.patch)
download | inline diff:
From 2d358ac48c94520628682df1f36661fd24292e76 Mon Sep 17 00:00:00 2001
From: Ayush Tiwari <[email protected]>
Date: Fri, 10 Apr 2026 19:17:12 +0530
Subject: [PATCH] Fix NULL dereference in pg_get_database_ddl()
pg_get_database_ddl_internal() can dereference a NULL pointer when
pg_database.dattablespace points to a tablespace OID that no longer
exists.
The immediate issue is that get_tablespace_name() may return NULL, but
the result is passed directly to pg_strcasecmp():
spcname = get_tablespace_name(dbform->dattablespace);
if (pg_strcasecmp(spcname, "pg_default") != 0)
...
That leads to a backend crash (SIGSEGV).
This function was introduced by commit a4f774cf1c7.
The patch fixes this by checking for NULL before calling
pg_strcasecmp(). In that case, pg_get_database_ddl() simply omits the
TABLESPACE clause.
A regression test is added in database_ddl.sql that exercises this
case by setting dattablespace to a non-existent OID and verifying that
the function returns successfully without crashing.
---
src/backend/utils/adt/ddlutils.c | 2 +-
src/test/regress/expected/database_ddl.out | 22 ++++++++++++++++++++++
src/test/regress/sql/database_ddl.sql | 17 +++++++++++++++++
3 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/src/backend/utils/adt/ddlutils.c b/src/backend/utils/adt/ddlutils.c
index b16c277d000..c5885b18958 100644
--- a/src/backend/utils/adt/ddlutils.c
+++ b/src/backend/utils/adt/ddlutils.c
@@ -976,7 +976,7 @@ pg_get_database_ddl_internal(Oid dbid, bool pretty,
{
char *spcname = get_tablespace_name(dbform->dattablespace);
- if (pg_strcasecmp(spcname, "pg_default") != 0)
+ if (spcname != NULL && pg_strcasecmp(spcname, "pg_default") != 0)
append_ddl_option(&buf, pretty, 4, "TABLESPACE = %s",
quote_identifier(spcname));
}
diff --git a/src/test/regress/expected/database_ddl.out b/src/test/regress/expected/database_ddl.out
index 97657e52cfa..00ec1bdc1f9 100644
--- a/src/test/regress/expected/database_ddl.out
+++ b/src/test/regress/expected/database_ddl.out
@@ -83,6 +83,28 @@ ERROR: permission denied for database regression_database_ddl
RESET ROLE;
GRANT CONNECT ON DATABASE regression_database_ddl TO PUBLIC;
DROP ROLE regress_db_ddl_noaccess;
+-- Test for dropped tablespace: dattablespace pointing to a non-existent OID
+-- should not crash (see commit fixing NULL deref in pg_get_database_ddl_internal)
+CREATE DATABASE regression_database_ddl2
+ ENCODING utf8 LC_COLLATE "C" LC_CTYPE "C" TEMPLATE template0
+ OWNER regress_datdba;
+SET allow_system_table_mods = on;
+UPDATE pg_database SET dattablespace = 99999
+ WHERE datname = 'regression_database_ddl2';
+RESET allow_system_table_mods;
+SELECT ddl_filter(pg_get_database_ddl) FROM pg_get_database_ddl('regression_database_ddl2');
+ ddl_filter
+---------------------------------------------------------------------------------------
+ CREATE DATABASE regression_database_ddl2 WITH TEMPLATE = template0 ENCODING = 'UTF8';
+ ALTER DATABASE regression_database_ddl2 OWNER TO regress_datdba;
+(2 rows)
+
+SET allow_system_table_mods = on;
+UPDATE pg_database SET dattablespace =
+ (SELECT oid FROM pg_tablespace WHERE spcname = 'pg_default')
+ WHERE datname = 'regression_database_ddl2';
+RESET allow_system_table_mods;
+DROP DATABASE regression_database_ddl2;
DROP DATABASE regression_database_ddl;
DROP FUNCTION ddl_filter(text);
DROP ROLE regress_datdba;
diff --git a/src/test/regress/sql/database_ddl.sql b/src/test/regress/sql/database_ddl.sql
index 89753ac6411..0792a4a9f35 100644
--- a/src/test/regress/sql/database_ddl.sql
+++ b/src/test/regress/sql/database_ddl.sql
@@ -61,6 +61,23 @@ RESET ROLE;
GRANT CONNECT ON DATABASE regression_database_ddl TO PUBLIC;
DROP ROLE regress_db_ddl_noaccess;
+-- Test for dropped tablespace: dattablespace pointing to a non-existent OID
+-- should not crash (see commit fixing NULL deref in pg_get_database_ddl_internal)
+CREATE DATABASE regression_database_ddl2
+ ENCODING utf8 LC_COLLATE "C" LC_CTYPE "C" TEMPLATE template0
+ OWNER regress_datdba;
+SET allow_system_table_mods = on;
+UPDATE pg_database SET dattablespace = 99999
+ WHERE datname = 'regression_database_ddl2';
+RESET allow_system_table_mods;
+SELECT ddl_filter(pg_get_database_ddl) FROM pg_get_database_ddl('regression_database_ddl2');
+SET allow_system_table_mods = on;
+UPDATE pg_database SET dattablespace =
+ (SELECT oid FROM pg_tablespace WHERE spcname = 'pg_default')
+ WHERE datname = 'regression_database_ddl2';
+RESET allow_system_table_mods;
+DROP DATABASE regression_database_ddl2;
+
DROP DATABASE regression_database_ddl;
DROP FUNCTION ddl_filter(text);
DROP ROLE regress_datdba;
--
2.34.1
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: [PATCH] Fix NULL dereference in pg_get_database_ddl()
2026-04-10 13:57 [PATCH] Fix NULL dereference in pg_get_database_ddl() Ayush Tiwari <[email protected]>
@ 2026-04-13 09:32 ` David Rowley <[email protected]>
2026-04-13 12:53 ` Re: [PATCH] Fix NULL dereference in pg_get_database_ddl() Ayush Tiwari <[email protected]>
1 sibling, 1 reply; 4+ messages in thread
From: David Rowley @ 2026-04-13 09:32 UTC (permalink / raw)
To: Ayush Tiwari <[email protected]>; +Cc: pgsql-hackers
On Sat, 11 Apr 2026 at 01:58, Ayush Tiwari <[email protected]> wrote:
> Deterministic reproduction:
>
> CREATE DATABASE regression_testdb;
> SET allow_system_table_mods = on;
> UPDATE pg_database
> SET dattablespace = 99999
> WHERE datname = 'regression_testdb';
> RESET allow_system_table_mods;
>
> SELECT * FROM pg_get_database_ddl('regression_testdb');
>
> The attached patch fixes this by checking for NULL before calling
> pg_strcasecmp(). In that case, pg_get_database_ddl() simply omits the
> TABLESPACE clause.
Can you explain why this method of self-inflicted catalogue corruption
is any more important than any of the just-about-infinite other ways
there are of causing issues by manually updating the catalogue tables?
The typical response to this sort of thing can be seen in the thread
in [1], in particular, the response in [2].
David
[1] https://www.postgresql.org/message-id/[email protected]
[2] https://www.postgresql.org/message-id/1538113.1768921841%40sss.pgh.pa.us
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: [PATCH] Fix NULL dereference in pg_get_database_ddl()
2026-04-10 13:57 [PATCH] Fix NULL dereference in pg_get_database_ddl() Ayush Tiwari <[email protected]>
2026-04-13 09:32 ` Re: [PATCH] Fix NULL dereference in pg_get_database_ddl() David Rowley <[email protected]>
@ 2026-04-13 12:53 ` Ayush Tiwari <[email protected]>
0 siblings, 0 replies; 4+ messages in thread
From: Ayush Tiwari @ 2026-04-13 12:53 UTC (permalink / raw)
To: David Rowley <[email protected]>; +Cc: pgsql-hackers
Hi,
On Mon, 13 Apr 2026 at 15:02, David Rowley <[email protected]> wrote:
> On Sat, 11 Apr 2026 at 01:58, Ayush Tiwari <[email protected]>
> wrote:
> > Deterministic reproduction:
> >
> > CREATE DATABASE regression_testdb;
> > SET allow_system_table_mods = on;
> > UPDATE pg_database
> > SET dattablespace = 99999
> > WHERE datname = 'regression_testdb';
> > RESET allow_system_table_mods;
> >
> > SELECT * FROM pg_get_database_ddl('regression_testdb');
> >
> > The attached patch fixes this by checking for NULL before calling
> > pg_strcasecmp(). In that case, pg_get_database_ddl() simply omits the
> > TABLESPACE clause.
>
> Can you explain why this method of self-inflicted catalogue corruption
> is any more important than any of the just-about-infinite other ways
> there are of causing issues by manually updating the catalogue tables?
>
> The typical response to this sort of thing can be seen in the thread
> in [1], in particular, the response in [2].
>
> David
>
> [1]
> https://www.postgresql.org/message-id/[email protected]
> [2]
> https://www.postgresql.org/message-id/1538113.1768921841%40sss.pgh.pa.us
Thanks for the review.
I dug into this further and I don't think I can support the patch as a real
bug fix.
My original thought was that there might be a supported concurrent-DDL path
where
pg_get_database_ddl_internal() reads pg_database.dattablespace, and then a
concurrent change makes get_tablespace_name() return NULL before the second
lookup. I tried to reproduce that by widening the window and testing
ALTER DATABASE ... SET TABLESPACE together with DROP TABLESPACE, but I
could not
make it happen.
The reason seems to be that both catalog reads are governed by the same
statement-level CatalogSnapshot, so the function continues to see a
consistent
catalog view for the duration of the call. On the other side, DROP
TABLESPACE
also doesn't appear able to commit a state where pg_database still points
at a
vanished tablespace row through supported SQL, because the drop fails while
the
database directory is still present.
So at this point the remaining crash case seems to require a broken catalog
state,
which puts this in the same bucket as the precedent you cited rather than a
supported-path bug.
Given that, I don't think this patch is worth pursuing further in its
current
form. I'll drop it here unless I find a supported reproducer or a different,
stronger issue around the missing database/tablespace bookkeeping.
Thanks again for the push to investigate it more carefully.
Regards,
Ayush
^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2026-04-13 12:53 UTC | newest]
Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-04-10 13:57 [PATCH] Fix NULL dereference in pg_get_database_ddl() Ayush Tiwari <[email protected]>
2026-04-10 14:13 ` Ayush Tiwari <[email protected]>
2026-04-13 09:32 ` David Rowley <[email protected]>
2026-04-13 12:53 ` Ayush Tiwari <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox