Thread: pgsql: Fix misbehavior of DROP OWNED BY with duplicate polroles entries
Fix misbehavior of DROP OWNED BY with duplicate polroles entries. Ordinarily, a pg_policy.polroles array wouldn't list the same role more than once; but CREATE POLICY does not prevent that. If we perform DROP OWNED BY on a role that is listed more than once, RemoveRoleFromObjectPolicy either suffered an assertion failure or encountered a tuple-updated-by-self error. Rewrite it to cope correctly with duplicate entries, and add a CommandCounterIncrement call to prevent the other problem. Per discussion, there's other cleanup that ought to happen here, but this seems like the minimum essential fix. Per bug #17062 from Alexander Lakhin. It's been broken all along, so back-patch to all supported branches. Discussion: https://postgr.es/m/17062-11f471ae3199ca23@postgresql.org Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/d21fca084356946664bfce19d66d2df2bb873cbd Modified Files -------------- src/backend/commands/policy.c | 50 +++++++++++++++++-------------- src/test/regress/expected/rowsecurity.out | 9 ++++++ src/test/regress/sql/rowsecurity.sql | 10 +++++++ 3 files changed, 46 insertions(+), 23 deletions(-)
Re: pgsql: Fix misbehavior of DROP OWNED BY with duplicate polroles entries
From
Michael Paquier
Date:
On Fri, Jun 18, 2021 at 10:00:33PM +0000, Tom Lane wrote: > Fix misbehavior of DROP OWNED BY with duplicate polroles entries. > > Ordinarily, a pg_policy.polroles array wouldn't list the same role > more than once; but CREATE POLICY does not prevent that. If we > perform DROP OWNED BY on a role that is listed more than once, > RemoveRoleFromObjectPolicy either suffered an assertion failure > or encountered a tuple-updated-by-self error. Rewrite it to cope > correctly with duplicate entries, and add a CommandCounterIncrement > call to prevent the other problem. > > Per discussion, there's other cleanup that ought to happen here, > but this seems like the minimum essential fix. A nit here. + /* If any roles remain, update the policy entry. */ + if (num_roles > 0) + { /* This is the array for the new tuple */ This indentation is incorrect. -- Michael
Attachment
Michael Paquier <michael@paquier.xyz> writes: > This indentation is incorrect. Yeah, I intentionally didn't reindent the code that's now inside the inner if-block. It would have made back-patching more painful (because that chunk isn't identical across branches). And I think we're going to be rewriting that whole function in a little bit, so I see no need to be very fussy about how pretty the intermediate state is. I just pushed that because I didn't want to ship beta2 with a known crasher bug. regards, tom lane