public inbox for [email protected]
help / color / mirror / Atom feedpg_init_privs corruption.
2+ messages / 2 participants
[nested] [flat]
* pg_init_privs corruption.
@ 2023-02-17 16:31 Kirill Reshke <[email protected]>
0 siblings, 0 replies; 2+ messages in thread
From: Kirill Reshke @ 2023-02-17 16:31 UTC (permalink / raw)
To: [email protected]
Hi hackers!
Recently we faced a problem with one of our production clusters. Problem
was with pg_upgrade,
the reason was an invalid pg_dump of cluster schema. in pg_dump sql there
was strange records like
REVOKE SELECT,INSERT,DELETE,UPDATE ON TABLE *relation* FROM "144841";
but there is no role "144841"
We did dig in, and it turns out that 144841 was OID of previously-deleted
role.
I have reproduced issue using simple test extension yoext(1).
SQL script:
create role user1;
ALTER DEFAULT PRIVILEGES IN SCHEMA public GRANT select ON TABLES TO user1;
create extension yoext;
drop owned by user1;
select * from pg_init_privs where privtype = 'e';
drop role user1;
select * from pg_init_privs where privtype = 'e';
result of execution (executed on fest master from commit
17feb6a566b77bf62ca453dec215adcc71755c20):
psql (16devel)
Type "help" for help.
postgres=#
postgres=#
postgres=# create role user1;
CREATE ROLE
postgres=# ALTER DEFAULT PRIVILEGES IN SCHEMA public GRANT select ON TABLES
TO user1;
ALTER DEFAULT PRIVILEGES
postgres=# create extension yobaext ;
CREATE EXTENSION
postgres=# drop owned by user1;
DROP OWNED
postgres=# select * from pg_init_privs where privtype = 'e';
objoid | classoid | objsubid | privtype | initprivs
--------+----------+----------+----------+---------------------------------------------------
16387 | 1259 | 0 | e |
{reshke=arwdDxtm/reshke,user1=r/reshke,=r/reshke}
(1 row)
postgres=# drop role user1;
DROP ROLE
postgres=# select * from pg_init_privs where privtype = 'e';
objoid | classoid | objsubid | privtype | initprivs
--------+----------+----------+----------+---------------------------------------------------
16387 | 1259 | 0 | e |
{reshke=arwdDxtm/reshke,16384=r/reshke,=r/reshke}
(1 row)
As you can see, after drop role there is invalid records in pg_init_privs
system relation. After this, pg_dump generate sql statements, some of which
are based on content of pg_init_privs, resulting in invalid dump.
PFA fix.
The idea of fix is simply drop records from pg_init_privs while dropping
role.
Records with grantor of grantee equal to oid of dropped role will erase.
after that, pg_dump works ok.
Implementation comment: i failed to find proper way to alloc acl array, so
defined some acl.c internal function `allocacl` in header. Need to improve
this somehow.
[1] yoext https://github.com/reshke/yoext/
Attachments:
[application/octet-stream] v1-0001-Fix-pg_init_prevs-corruption.patch (4.9K, ../../CADVKa1Wq7FcXy1xyqN-26_2TnW5Lva9A8S+J1kvdVM08E3hGBw@mail.gmail.com/3-v1-0001-Fix-pg_init_prevs-corruption.patch)
download | inline diff:
From 4af27a0a9c5eacab7e580fc015caec1e19586111 Mon Sep 17 00:00:00 2001
From: reshke kirill <[email protected]>
Date: Fri, 17 Feb 2023 16:05:37 +0000
Subject: [PATCH v1] Fix pg_init_prevs corruption.
Drop some acl items from initprivs in pg_init_prevs
while DROP ROLE.
---
src/backend/catalog/dependency.c | 90 ++++++++++++++++++++++++++++++++
src/backend/commands/user.c | 5 ++
src/backend/utils/adt/acl.c | 3 +-
src/include/catalog/dependency.h | 2 +
src/include/utils/acl.h | 2 +
5 files changed, 100 insertions(+), 2 deletions(-)
diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c
index f8a136ba0a..6bdfab5e06 100644
--- a/src/backend/catalog/dependency.c
+++ b/src/backend/catalog/dependency.c
@@ -3009,3 +3009,93 @@ DeleteInitPrivs(const ObjectAddress *object)
table_close(relation, RowExclusiveLock);
}
+
+
+
+#define ARRNELEMS(x) ArrayGetNItems( ARR_NDIM(x), ARR_DIMS(x))
+
+/*
+ * modify pg_init_prevs ACL for extension objects on role delete
+ */
+void
+DeleteInitPrivsRefs(Oid roleoid)
+{
+ Relation relation;
+ ScanKeyData key[1];
+ SysScanDesc scan;
+ HeapTuple oldtuple;
+ HeapTuple newtuple;
+ Datum initprivs;
+ AclItem * acls;
+ Acl *iniprivsacl;
+ Acl *new_acl;
+ bool initprvis_isnull;
+ bool found;
+ int dim;
+ int new_acl_sz;
+
+ Datum values[Natts_pg_init_privs] = {0};
+ bool nulls[Natts_pg_init_privs] = {0};
+ bool replaces[Natts_pg_init_privs] = {0};
+
+
+ relation = table_open(InitPrivsRelationId, RowExclusiveLock);
+
+ ScanKeyInit(&key[0],
+ Anum_pg_init_privs_privtype,
+ BTEqualStrategyNumber, F_CHAREQ,
+ INITPRIVS_EXTENSION);
+
+ scan = systable_beginscan(relation, InitPrivsObjIndexId, false,
+ NULL, 1, key);
+
+ while (HeapTupleIsValid(oldtuple = systable_getnext(scan))) {
+ initprivs = heap_getattr(oldtuple, Anum_pg_init_privs_initprivs, relation->rd_att, &initprvis_isnull);
+
+ if (initprvis_isnull) {
+ continue;
+ }
+
+ iniprivsacl = DatumGetAclP(initprivs);
+ found = false;
+
+ acls = (AclItem *) ARR_DATA_PTR(iniprivsacl);
+ dim = ARRNELEMS(iniprivsacl);
+ new_acl_sz = 0;
+
+ for (int i = 0; i < dim; ++ i) {
+ if (acls[i].ai_grantee == roleoid || acls[i].ai_grantor == roleoid) {
+ found = true;
+ continue;
+ }
+ new_acl_sz++;
+ }
+
+ if (!found) {
+ continue;
+ }
+
+ new_acl = allocacl(new_acl_sz);
+
+ new_acl_sz = 0;
+
+ for (int i = 0; i < dim; ++ i) {
+ if (acls[i].ai_grantee == roleoid || acls[i].ai_grantor == roleoid) {
+ continue;
+ }
+ ((AclItem *) ARR_DATA_PTR(new_acl))[new_acl_sz++] = acls[i];
+ }
+
+ replaces[Anum_pg_init_privs_initprivs - 1] = true;
+ values[Anum_pg_init_privs_initprivs - 1] = PointerGetDatum(new_acl);
+
+ newtuple = heap_modify_tuple(oldtuple, RelationGetDescr(relation),
+ values, nulls, replaces);
+
+ CatalogTupleUpdate(relation, &newtuple->t_self, newtuple);
+ }
+
+ systable_endscan(scan);
+
+ table_close(relation, RowExclusiveLock);
+}
\ No newline at end of file
diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
index 3a92e930c0..49616439ef 100644
--- a/src/backend/commands/user.c
+++ b/src/backend/commands/user.c
@@ -1283,6 +1283,11 @@ DropRole(DropRoleStmt *stmt)
* Remove settings for this role.
*/
DropSetting(InvalidOid, roleid);
+
+ /*
+ * Remove pg_init_privs enries for that role
+ */
+ DeleteInitPrivsRefs(roleid);
}
/*
diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c
index 8f7522d103..57485bb4dc 100644
--- a/src/backend/utils/adt/acl.c
+++ b/src/backend/utils/adt/acl.c
@@ -83,7 +83,6 @@ static uint32 cached_db_hash;
static const char *getid(const char *s, char *n, Node *escontext);
static void putid(char *p, const char *s);
-static Acl *allocacl(int n);
static void check_acl(const Acl *acl);
static const char *aclparse(const char *s, AclItem *aip, Node *escontext);
static bool aclitem_match(const AclItem *a1, const AclItem *a2);
@@ -399,7 +398,7 @@ aclparse(const char *s, AclItem *aip, Node *escontext)
* RETURNS:
* the new Acl
*/
-static Acl *
+Acl *
allocacl(int n)
{
Acl *new_acl;
diff --git a/src/include/catalog/dependency.h b/src/include/catalog/dependency.h
index ffd5e9dc82..d2710968ca 100644
--- a/src/include/catalog/dependency.h
+++ b/src/include/catalog/dependency.h
@@ -267,4 +267,6 @@ extern void shdepDropOwned(List *roleids, DropBehavior behavior);
extern void shdepReassignOwned(List *roleids, Oid newrole);
+extern void DeleteInitPrivsRefs(Oid roleoid);
+
#endif /* DEPENDENCY_H */
diff --git a/src/include/utils/acl.h b/src/include/utils/acl.h
index f8e1238fa2..dc8e3227a3 100644
--- a/src/include/utils/acl.h
+++ b/src/include/utils/acl.h
@@ -276,4 +276,6 @@ extern bool object_ownercheck(Oid classid, Oid objectid, Oid roleid);
extern bool has_createrole_privilege(Oid roleid);
extern bool has_bypassrls_privilege(Oid roleid);
+extern Acl * allocacl(int n);
+
#endif /* ACL_H */
--
2.25.1
^ permalink raw reply [nested|flat] 2+ messages in thread
* [PATCH v1 1/1] clean up AdjustUpgrade.pm after pg_upgrade min version bump
@ 2026-07-08 21:39 Nathan Bossart <[email protected]>
0 siblings, 0 replies; 2+ messages in thread
From: Nathan Bossart @ 2026-07-08 21:39 UTC (permalink / raw)
---
.../perl/PostgreSQL/Test/AdjustUpgrade.pm | 207 +-----------------
1 file changed, 5 insertions(+), 202 deletions(-)
diff --git a/src/test/perl/PostgreSQL/Test/AdjustUpgrade.pm b/src/test/perl/PostgreSQL/Test/AdjustUpgrade.pm
index 878fcc16a9e..04844298ab3 100644
--- a/src/test/perl/PostgreSQL/Test/AdjustUpgrade.pm
+++ b/src/test/perl/PostgreSQL/Test/AdjustUpgrade.pm
@@ -199,15 +199,10 @@ sub adjust_database_contents
'drop operator if exists #@%# (bigint,NONE)');
# get rid of dblink's dependencies on regress.so
- my $regrdb =
- $old_version le '9.4'
- ? 'contrib_regression'
- : 'contrib_regression_dblink';
-
- if ($dbnames{$regrdb})
+ if ($dbnames{'contrib_regression_dblink'})
{
_add_st(
- $result, $regrdb,
+ $result, 'contrib_regression_dblink',
'drop function if exists public.putenv(text)',
'drop function if exists public.wait_pid(integer)');
}
@@ -250,7 +245,7 @@ sub adjust_database_contents
}
# this table had OIDs too, but we'll just drop it
- if ($old_version >= 10 && $dbnames{'contrib_regression_postgres_fdw'})
+ if ($dbnames{'contrib_regression_postgres_fdw'})
{
_add_st(
$result,
@@ -276,43 +271,13 @@ sub adjust_database_contents
'drop function if exists public.funny_dup17()');
}
- # version-0 C functions are no longer supported
- if ($old_version < 10)
- {
- _add_st($result, 'regression',
- 'drop function oldstyle_length(integer, text)');
- }
-
- if ($old_version lt '9.5')
- {
- # cope with changes of underlying functions
- _add_st(
- $result,
- 'regression',
- 'drop operator @#@ (NONE, bigint)',
- 'CREATE OPERATOR @#@ ('
- . 'PROCEDURE = factorial, RIGHTARG = bigint )',
- 'drop aggregate public.array_cat_accum(anyarray)',
- 'CREATE AGGREGATE array_larger_accum (anyarray) ' . ' ( '
- . ' sfunc = array_larger, '
- . ' stype = anyarray, '
- . ' initcond = $${}$$ ' . ' ) ');
-
- # "=>" is no longer valid as an operator name
- _add_st($result, 'regression',
- 'drop operator if exists public.=> (bigint, NONE)');
- }
-
# Version 19 changed the output format of pg_lsn. To avoid output
# differences, set all pg_lsn columns to NULL if the old version is
# older than 19.
if ($old_version < 19)
{
- if ($old_version >= '9.5')
- {
- _add_st($result, 'regression',
- "update brintest set lsncol = NULL");
- }
+ _add_st($result, 'regression',
+ "update brintest set lsncol = NULL");
if ($old_version >= 12)
{
@@ -426,107 +391,6 @@ sub adjust_old_dumpfile
/$1 EXECUTE FUNCTION/mgx;
}
- # During pg_upgrade, we reindex hash indexes if the source is pre-v10.
- # This may change their tables' relallvisible values, so don't compare
- # those.
- if ($old_version < 10)
- {
- $dump =~ s/
- (^SELECT\s\*\sFROM\spg_catalog\.pg_restore_relation_stats\(
- [^;]*'relation',\s'public\.hash_[a-z0-9]*_heap'::regclass,
- [^;]*'relallvisible',)\s'\d+'::integer
- /$1 ''::integer/mgx;
- }
-
- if ($old_version lt '9.6')
- {
- # adjust some places where we don't print so many parens anymore
-
- my $prefix =
- "'New York'\tnew & york | big & apple | nyc\t'new' & 'york'\t";
- my $orig = "( 'new' & 'york' | 'big' & 'appl' ) | 'nyc'";
- my $repl = "'new' & 'york' | 'big' & 'appl' | 'nyc'";
- $dump =~ s/(?<=^\Q$prefix\E)\Q$orig\E/$repl/mg;
-
- $prefix =
- "'Sanct Peter'\tPeterburg | peter | 'Sanct Peterburg'\t'sanct' & 'peter'\t";
- $orig = "( 'peterburg' | 'peter' ) | 'sanct' & 'peterburg'";
- $repl = "'peterburg' | 'peter' | 'sanct' & 'peterburg'";
- $dump =~ s/(?<=^\Q$prefix\E)\Q$orig\E/$repl/mg;
- }
-
- if ($old_version lt '9.5')
- {
- # adjust some places where we don't print so many parens anymore
-
- my $prefix = "CONSTRAINT (?:sequence|copy)_con CHECK [(][(]";
- my $orig = "((x > 3) AND (y <> 'check failed'::text))";
- my $repl = "(x > 3) AND (y <> 'check failed'::text)";
- $dump =~ s/($prefix)\Q$orig\E/$1$repl/mg;
-
- $prefix = "CONSTRAINT insert_con CHECK [(][(]";
- $orig = "((x >= 3) AND (y <> 'check failed'::text))";
- $repl = "(x >= 3) AND (y <> 'check failed'::text)";
- $dump =~ s/($prefix)\Q$orig\E/$1$repl/mg;
-
- $orig = "DEFAULT ((-1) * currval('public.insert_seq'::regclass))";
- $repl =
- "DEFAULT ('-1'::integer * currval('public.insert_seq'::regclass))";
- $dump =~ s/\Q$orig\E/$repl/mg;
-
- my $expr =
- "(rsl.sl_color = rsh.slcolor) AND (rsl.sl_len_cm >= rsh.slminlen_cm)";
- $dump =~ s/WHERE \(\(\Q$expr\E\)/WHERE ($expr/g;
-
- $expr =
- "(rule_and_refint_t3.id3a = new.id3a) AND (rule_and_refint_t3.id3b = new.id3b)";
- $dump =~ s/WHERE \(\(\Q$expr\E\)/WHERE ($expr/g;
-
- $expr =
- "(rule_and_refint_t3_1.id3a = new.id3a) AND (rule_and_refint_t3_1.id3b = new.id3b)";
- $dump =~ s/WHERE \(\(\Q$expr\E\)/WHERE ($expr/g;
- }
-
- if ($old_version lt '9.3')
- {
- # CREATE VIEW/RULE statements were not pretty-printed before 9.3.
- # To cope, reduce all whitespace sequences within them to one space.
- # This must be done on both old and new dumps.
- $dump = _mash_view_whitespace($dump);
-
- # _mash_view_whitespace doesn't handle multi-command rules;
- # rather than trying to fix that, just hack the exceptions manually.
-
- my $prefix =
- "CREATE RULE rtest_sys_del AS ON DELETE TO public.rtest_system DO (DELETE FROM public.rtest_interface WHERE (rtest_interface.sysname = old.sysname);";
- my $line2 = " DELETE FROM public.rtest_admin";
- my $line3 = " WHERE (rtest_admin.sysname = old.sysname);";
- $dump =~
- s/(?<=\Q$prefix\E)\Q$line2$line3\E \);/\n$line2\n $line3\n);/mg;
-
- $prefix =
- "CREATE RULE rtest_sys_upd AS ON UPDATE TO public.rtest_system DO (UPDATE public.rtest_interface SET sysname = new.sysname WHERE (rtest_interface.sysname = old.sysname);";
- $line2 = " UPDATE public.rtest_admin SET sysname = new.sysname";
- $line3 = " WHERE (rtest_admin.sysname = old.sysname);";
- $dump =~
- s/(?<=\Q$prefix\E)\Q$line2$line3\E \);/\n$line2\n $line3\n);/mg;
-
- # and there's one place where pre-9.3 uses a different table alias
- $dump =~ s {^(CREATE\sRULE\srule_and_refint_t3_ins\sAS\s
- ON\sINSERT\sTO\spublic\.rule_and_refint_t3\s
- WHERE\s\(EXISTS\s\(SELECT\s1\sFROM\spublic\.rule_and_refint_t3)\s
- (WHERE\s\(\(rule_and_refint_t3)
- (\.id3a\s=\snew\.id3a\)\sAND\s\(rule_and_refint_t3)
- (\.id3b\s=\snew\.id3b\)\sAND\s\(rule_and_refint_t3)}
- {$1 rule_and_refint_t3_1 $2_1$3_1$4_1}mx;
-
- # Also fix old use of NATURAL JOIN syntax
- $dump =~ s {NATURAL JOIN public\.credit_card r}
- {JOIN public.credit_card r USING (cid)}mg;
- $dump =~ s {NATURAL JOIN public\.credit_usage r}
- {JOIN public.credit_usage r USING (cid)}mg;
- }
-
# Suppress blank lines, as some places in pg_dump emit more or fewer.
$dump =~ s/\n\n+/\n/g;
@@ -661,37 +525,6 @@ sub _mash_view_qualifiers
return $dump;
}
-
-# Internal subroutine to mangle whitespace within view/rule commands.
-# Any consecutive sequence of whitespace is reduced to one space.
-sub _mash_view_whitespace
-{
- my ($dump) = @_;
-
- foreach my $leader ('CREATE VIEW', 'CREATE RULE')
- {
- my @splitchunks = split $leader, $dump;
-
- $dump = shift(@splitchunks);
- foreach my $chunk (@splitchunks)
- {
- my @thischunks = split /;/, $chunk, 2;
- my $stmt = shift(@thischunks);
-
- # now $stmt is just the body of the CREATE VIEW/RULE
- $stmt =~ s/\s+/ /sg;
- # we also need to smash these forms for sub-selects and rules
- $stmt =~ s/\( SELECT/(SELECT/g;
- $stmt =~ s/\( INSERT/(INSERT/g;
- $stmt =~ s/\( UPDATE/(UPDATE/g;
- $stmt =~ s/\( DELETE/(DELETE/g;
-
- $dump .= $leader . $stmt . ';' . $thischunks[0];
- }
- }
- return $dump;
-}
-
=pod
=item adjust_new_dumpfile($old_version, $dump)
@@ -779,36 +612,6 @@ sub adjust_new_dumpfile
$dump =~ s/^SET default_table_access_method = heap;\n//mg;
}
- # During pg_upgrade, we reindex hash indexes if the source is pre-v10.
- # This may change their tables' relallvisible values, so don't compare
- # those.
- if ($old_version < 10)
- {
- $dump =~ s/
- (^SELECT\s\*\sFROM\spg_catalog\.pg_restore_relation_stats\(
- [^;]*'relation',\s'public\.hash_[a-z0-9]*_heap'::regclass,
- [^;]*'relallvisible',)\s'\d+'::integer
- /$1 ''::integer/mgx;
- }
-
- # dumps from pre-9.6 dblink may include redundant ACL settings
- if ($old_version lt '9.6')
- {
- my $comment =
- "-- Name: FUNCTION dblink_connect_u\(.*?\); Type: ACL; Schema: public; Owner: .*";
- my $sql =
- "REVOKE ALL ON FUNCTION public\.dblink_connect_u\(.*?\) FROM PUBLIC;";
- $dump =~ s/^--\n$comment\n--\n+$sql\n+//mg;
- }
-
- if ($old_version lt '9.3')
- {
- # CREATE VIEW/RULE statements were not pretty-printed before 9.3.
- # To cope, reduce all whitespace sequences within them to one space.
- # This must be done on both old and new dumps.
- $dump = _mash_view_whitespace($dump);
- }
-
# Suppress blank lines, as some places in pg_dump emit more or fewer.
$dump =~ s/\n\n+/\n/g;
--
2.50.1 (Apple Git-155)
--WhtY6+QncUkBAPlR--
^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2026-07-08 21:39 UTC | newest]
Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-02-17 16:31 pg_init_privs corruption. Kirill Reshke <[email protected]>
2026-07-08 21:39 [PATCH v1 1/1] clean up AdjustUpgrade.pm after pg_upgrade min version bump Nathan Bossart <[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