Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 16eff42

Browse files
committed
Fix pg_dumpall to cope with dangling OIDs in pg_auth_members.
There is a race condition between "GRANT role" and "DROP ROLE", which allows GRANT to install pg_auth_members entries that refer to dropped roles. (Commit 6566133 prevented that for the grantor field, but not for the granted or grantee roles.) We'll soon fix that, at least in HEAD, but pg_dumpall needs to cope with the situation in case of pre-existing inconsistency. As pg_dumpall stands, it will emit invalid commands like 'GRANT foo TO ""', which causes pg_upgrade to fail. Fix it to emit warnings and skip those GRANTs, instead. There was some discussion of removing the problem by changing dumpRoleMembership's query to use JOIN not LEFT JOIN, but that would result in silently ignoring such entries. It seems better to produce a warning. Pre-v16 branches already coped with dangling grantor OIDs by simply omitting the GRANTED BY clause. I left that behavior as-is, although it's somewhat inconsistent with the behavior of later branches. Reported-by: Virender Singla <virender.cse@gmail.com> Discussion: https://postgr.es/m/CAM6Zo8woa62ZFHtMKox6a4jb8qQ=w87R2L0K8347iE-juQL2EA@mail.gmail.com Backpatch-through: 13
1 parent 69c45ec commit 16eff42

File tree

1 file changed

+57
-9
lines changed

1 file changed

+57
-9
lines changed

src/bin/pg_dump/pg_dumpall.c

+57-9
Original file line numberDiff line numberDiff line change
@@ -966,6 +966,13 @@ dumpRoleMembership(PGconn *conn)
966966
total;
967967
bool dump_grantors;
968968
bool dump_grant_options;
969+
int i_role;
970+
int i_member;
971+
int i_grantor;
972+
int i_roleid;
973+
int i_memberid;
974+
int i_grantorid;
975+
int i_admin_option;
969976
int i_inherit_option;
970977
int i_set_option;
971978

@@ -975,6 +982,10 @@ dumpRoleMembership(PGconn *conn)
975982
* they didn't have ADMIN OPTION on the role, or a user that no longer
976983
* existed. To avoid dump and restore failures, don't dump the grantor
977984
* when talking to an old server version.
985+
*
986+
* Also, in older versions the roleid and/or member could be role OIDs
987+
* that no longer exist. If we find such cases, print a warning and skip
988+
* the entry.
978989
*/
979990
dump_grantors = (PQserverVersion(conn) >= 160000);
980991

@@ -986,8 +997,10 @@ dumpRoleMembership(PGconn *conn)
986997
/* Generate and execute query. */
987998
printfPQExpBuffer(buf, "SELECT ur.rolname AS role, "
988999
"um.rolname AS member, "
989-
"ug.oid AS grantorid, "
9901000
"ug.rolname AS grantor, "
1001+
"a.roleid AS roleid, "
1002+
"a.member AS memberid, "
1003+
"a.grantor AS grantorid, "
9911004
"a.admin_option");
9921005
if (dump_grant_options)
9931006
appendPQExpBufferStr(buf, ", a.inherit_option, a.set_option");
@@ -996,8 +1009,15 @@ dumpRoleMembership(PGconn *conn)
9961009
"LEFT JOIN %s um on um.oid = a.member "
9971010
"LEFT JOIN %s ug on ug.oid = a.grantor "
9981011
"WHERE NOT (ur.rolname ~ '^pg_' AND um.rolname ~ '^pg_')"
999-
"ORDER BY 1,2,4", role_catalog, role_catalog, role_catalog);
1012+
"ORDER BY 1,2,3", role_catalog, role_catalog, role_catalog);
10001013
res = executeQuery(conn, buf->data);
1014+
i_role = PQfnumber(res, "role");
1015+
i_member = PQfnumber(res, "member");
1016+
i_grantor = PQfnumber(res, "grantor");
1017+
i_roleid = PQfnumber(res, "roleid");
1018+
i_memberid = PQfnumber(res, "memberid");
1019+
i_grantorid = PQfnumber(res, "grantorid");
1020+
i_admin_option = PQfnumber(res, "admin_option");
10011021
i_inherit_option = PQfnumber(res, "inherit_option");
10021022
i_set_option = PQfnumber(res, "set_option");
10031023

@@ -1021,24 +1041,32 @@ dumpRoleMembership(PGconn *conn)
10211041
total = PQntuples(res);
10221042
while (start < total)
10231043
{
1024-
char *role = PQgetvalue(res, start, 0);
1044+
char *role = PQgetvalue(res, start, i_role);
10251045
int i;
10261046
bool *done;
10271047
int remaining;
10281048
int prev_remaining = 0;
10291049
rolename_hash *ht;
10301050

1051+
/* If we hit a null roleid, we're done (nulls sort to the end). */
1052+
if (PQgetisnull(res, start, i_role))
1053+
{
1054+
/* translator: %s represents a numeric role OID */
1055+
pg_log_warning("found orphaned pg_auth_members entry for role %s",
1056+
PQgetvalue(res, start, i_roleid));
1057+
break;
1058+
}
1059+
10311060
/* All memberships for a single role should be adjacent. */
10321061
for (end = start; end < total; ++end)
10331062
{
10341063
char *otherrole;
10351064

1036-
otherrole = PQgetvalue(res, end, 0);
1065+
otherrole = PQgetvalue(res, end, i_role);
10371066
if (strcmp(role, otherrole) != 0)
10381067
break;
10391068
}
10401069

1041-
role = PQgetvalue(res, start, 0);
10421070
remaining = end - start;
10431071
done = pg_malloc0(remaining * sizeof(bool));
10441072
ht = rolename_create(remaining, NULL);
@@ -1078,10 +1106,30 @@ dumpRoleMembership(PGconn *conn)
10781106
if (done[i - start])
10791107
continue;
10801108

1081-
member = PQgetvalue(res, i, 1);
1082-
grantorid = PQgetvalue(res, i, 2);
1083-
grantor = PQgetvalue(res, i, 3);
1084-
admin_option = PQgetvalue(res, i, 4);
1109+
/* Complain about, then ignore, entries with orphaned OIDs. */
1110+
if (PQgetisnull(res, i, i_member))
1111+
{
1112+
/* translator: %s represents a numeric role OID */
1113+
pg_log_warning("found orphaned pg_auth_members entry for role %s",
1114+
PQgetvalue(res, i, i_memberid));
1115+
done[i - start] = true;
1116+
--remaining;
1117+
continue;
1118+
}
1119+
if (PQgetisnull(res, i, i_grantor))
1120+
{
1121+
/* translator: %s represents a numeric role OID */
1122+
pg_log_warning("found orphaned pg_auth_members entry for role %s",
1123+
PQgetvalue(res, i, i_grantorid));
1124+
done[i - start] = true;
1125+
--remaining;
1126+
continue;
1127+
}
1128+
1129+
member = PQgetvalue(res, i, i_member);
1130+
grantor = PQgetvalue(res, i, i_grantor);
1131+
grantorid = PQgetvalue(res, i, i_grantorid);
1132+
admin_option = PQgetvalue(res, i, i_admin_option);
10851133
if (dump_grant_options)
10861134
set_option = PQgetvalue(res, i, i_set_option);
10871135

0 commit comments

Comments
 (0)