From e3f1c24b992acb88e4ccf33118640aee4b11dd47 Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Mon, 9 Mar 2015 17:00:43 -0300 Subject: [PATCH] Fix crasher bugs in previous commit ALTER DEFAULT PRIVILEGES was trying to decode the list of roles in the FOR clause as a list of names rather than of RoleSpecs; and the IN clause in CREATE ROLE was doing the same thing. This was evidenced by crashes on some buildfarm machines, though on my platform this doesn't cause a failure by mere chance; I can reproduce the failures only by adding some padding in struct RoleSpecs. Fix by dereferencing those lists as being of RoleSpecs, not string Values. --- src/backend/catalog/aclchk.c | 20 ++++++++++---------- src/backend/commands/user.c | 8 ++++++-- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c index 6c8780f794..8e75c27920 100644 --- a/src/backend/catalog/aclchk.c +++ b/src/backend/catalog/aclchk.c @@ -858,9 +858,9 @@ ExecAlterDefaultPrivilegesStmt(AlterDefaultPrivilegesStmt *stmt) GrantStmt *action = stmt->action; InternalDefaultACL iacls; ListCell *cell; - List *rolenames = NIL; + List *rolespecs = NIL; List *nspnames = NIL; - DefElem *drolenames = NULL; + DefElem *drolespecs = NULL; DefElem *dnspnames = NULL; AclMode all_privileges; const char *errormsg; @@ -880,11 +880,11 @@ ExecAlterDefaultPrivilegesStmt(AlterDefaultPrivilegesStmt *stmt) } else if (strcmp(defel->defname, "roles") == 0) { - if (drolenames) + if (drolespecs) ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), errmsg("conflicting or redundant options"))); - drolenames = defel; + drolespecs = defel; } else elog(ERROR, "option \"%s\" not recognized", defel->defname); @@ -892,8 +892,8 @@ ExecAlterDefaultPrivilegesStmt(AlterDefaultPrivilegesStmt *stmt) if (dnspnames) nspnames = (List *) dnspnames->arg; - if (drolenames) - rolenames = (List *) drolenames->arg; + if (drolespecs) + rolespecs = (List *) drolespecs->arg; /* Prepare the InternalDefaultACL representation of the statement */ /* roleid to be filled below */ @@ -996,7 +996,7 @@ ExecAlterDefaultPrivilegesStmt(AlterDefaultPrivilegesStmt *stmt) } } - if (rolenames == NIL) + if (rolespecs == NIL) { /* Set permissions for myself */ iacls.roleid = GetUserId(); @@ -1008,11 +1008,11 @@ ExecAlterDefaultPrivilegesStmt(AlterDefaultPrivilegesStmt *stmt) /* Look up the role OIDs and do permissions checks */ ListCell *rolecell; - foreach(rolecell, rolenames) + foreach(rolecell, rolespecs) { - char *rolename = strVal(lfirst(rolecell)); + RoleSpec *rolespec = lfirst(rolecell); - iacls.roleid = get_role_oid(rolename, false); + iacls.roleid = get_rolespec_oid((Node *) rolespec, false); /* * We insist that calling user be a member of each target role. If diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c index c14465eb87..75f1b3cd4f 100644 --- a/src/backend/commands/user.c +++ b/src/backend/commands/user.c @@ -429,13 +429,17 @@ CreateRole(CreateRoleStmt *stmt) */ foreach(item, addroleto) { - char *oldrolename = strVal(lfirst(item)); - Oid oldroleid = get_role_oid(oldrolename, false); + RoleSpec *oldrole = lfirst(item); + HeapTuple oldroletup = get_rolespec_tuple((Node *) oldrole); + Oid oldroleid = HeapTupleGetOid(oldroletup); + char *oldrolename = NameStr(((Form_pg_authid) GETSTRUCT(oldroletup))->rolname); AddRoleMems(oldrolename, oldroleid, list_make1(makeString(stmt->role)), list_make1_oid(roleid), GetUserId(), false); + + ReleaseSysCache(oldroletup); } /*