agora inbox for pgsql-committers@postgresql.orghelp / color / mirror / Atom feed
pgsql: Fix unsafe order of operations in foreign-table DDL commands. 7+ messages / 1 participants [nested] [flat]
* pgsql: Fix unsafe order of operations in foreign-table DDL commands. @ 2011-08-14 19:41 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 7+ messages in thread From: Tom Lane @ 2011-08-14 19:41 UTC (permalink / raw) To: pgsql-committers Fix unsafe order of operations in foreign-table DDL commands. When updating or deleting a system catalog tuple, it's necessary to acquire RowExclusiveLock on the catalog before looking up the tuple; otherwise a concurrent VACUUM FULL on the catalog might move the tuple to a different TID before we can apply the update. Coding patterns that find the tuple via a table scan aren't at risk here, but when obtaining the tuple from a catalog cache, correct ordering is important; and several routines in foreigncmds.c got it wrong. Noted while running the regression tests in parallel with VACUUM FULL of assorted system catalogs. For consistency I moved all the heap_open calls to the starts of their functions, including a couple for which there was no actual bug. Back-patch to 8.4 where foreigncmds.c was added. Branch ------ REL8_4_STABLE Details ------- http://git.postgresql.org/pg/commitdiff/3c96f5c647852d1c77f9854cf1f78dfb01af8631 Modified Files -------------- src/backend/commands/foreigncmds.c | 31 ++++++++++++++----------------- 1 files changed, 14 insertions(+), 17 deletions(-) ^ permalink raw reply [nested|flat] 7+ messages in thread
* pgsql: Fix unsafe order of operations in foreign-table DDL commands. @ 2011-08-14 19:41 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 7+ messages in thread From: Tom Lane @ 2011-08-14 19:41 UTC (permalink / raw) To: pgsql-committers Fix unsafe order of operations in foreign-table DDL commands. When updating or deleting a system catalog tuple, it's necessary to acquire RowExclusiveLock on the catalog before looking up the tuple; otherwise a concurrent VACUUM FULL on the catalog might move the tuple to a different TID before we can apply the update. Coding patterns that find the tuple via a table scan aren't at risk here, but when obtaining the tuple from a catalog cache, correct ordering is important; and several routines in foreigncmds.c got it wrong. Noted while running the regression tests in parallel with VACUUM FULL of assorted system catalogs. For consistency I moved all the heap_open calls to the starts of their functions, including a couple for which there was no actual bug. Back-patch to 8.4 where foreigncmds.c was added. Branch ------ REL9_1_STABLE Details ------- http://git.postgresql.org/pg/commitdiff/148f321eb4fe9030378ad21ba1be132084a9ab8f Modified Files -------------- src/backend/commands/foreigncmds.c | 39 ++++++++++++++++------------------- 1 files changed, 18 insertions(+), 21 deletions(-) ^ permalink raw reply [nested|flat] 7+ messages in thread
* pgsql: Fix unsafe order of operations in foreign-table DDL commands. @ 2011-08-14 19:41 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 7+ messages in thread From: Tom Lane @ 2011-08-14 19:41 UTC (permalink / raw) To: pgsql-committers Fix unsafe order of operations in foreign-table DDL commands. When updating or deleting a system catalog tuple, it's necessary to acquire RowExclusiveLock on the catalog before looking up the tuple; otherwise a concurrent VACUUM FULL on the catalog might move the tuple to a different TID before we can apply the update. Coding patterns that find the tuple via a table scan aren't at risk here, but when obtaining the tuple from a catalog cache, correct ordering is important; and several routines in foreigncmds.c got it wrong. Noted while running the regression tests in parallel with VACUUM FULL of assorted system catalogs. For consistency I moved all the heap_open calls to the starts of their functions, including a couple for which there was no actual bug. Back-patch to 8.4 where foreigncmds.c was added. Branch ------ master Details ------- http://git.postgresql.org/pg/commitdiff/52994e9e5686b10a92bc93ec0f4e15c7fbc18242 Modified Files -------------- src/backend/commands/foreigncmds.c | 39 ++++++++++++++++------------------- 1 files changed, 18 insertions(+), 21 deletions(-) ^ permalink raw reply [nested|flat] 7+ messages in thread
* pgsql: Fix unsafe order of operations in foreign-table DDL commands. @ 2011-08-14 19:41 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 7+ messages in thread From: Tom Lane @ 2011-08-14 19:41 UTC (permalink / raw) To: pgsql-committers Fix unsafe order of operations in foreign-table DDL commands. When updating or deleting a system catalog tuple, it's necessary to acquire RowExclusiveLock on the catalog before looking up the tuple; otherwise a concurrent VACUUM FULL on the catalog might move the tuple to a different TID before we can apply the update. Coding patterns that find the tuple via a table scan aren't at risk here, but when obtaining the tuple from a catalog cache, correct ordering is important; and several routines in foreigncmds.c got it wrong. Noted while running the regression tests in parallel with VACUUM FULL of assorted system catalogs. For consistency I moved all the heap_open calls to the starts of their functions, including a couple for which there was no actual bug. Back-patch to 8.4 where foreigncmds.c was added. Branch ------ REL9_0_STABLE Details ------- http://git.postgresql.org/pg/commitdiff/5707f355593c91a6c866835a7c55eabaede23628 Modified Files -------------- src/backend/commands/foreigncmds.c | 31 ++++++++++++++----------------- 1 files changed, 14 insertions(+), 17 deletions(-) ^ permalink raw reply [nested|flat] 7+ messages in thread
* pgsql: Fix unsafe order of operations in ResourceOwnerReleaseAll(). @ 2026-06-22 22:03 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 7+ messages in thread From: Tom Lane @ 2026-06-22 22:03 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix unsafe order of operations in ResourceOwnerReleaseAll(). This function called the resource-kind-specific ReleaseResource() method for each item before deleting that item from the resowner. That's backwards from the ordering in ResourceOwnerReleaseAllOfKind, and it's not very safe. If ReleaseResource throws an error then the subsequent abort cleanup will come back here and try to release that item again, possibly leading to a double-free or similar crash, and in any case risking an infinite error cleanup loop. This mistake explains why the pgcrypto bug just fixed in 80bb0ebcc led to a crash rather than something more benign. Remove the item from the resowner, then call ReleaseResource, matching the way things were done before b8bff07da. If there is a problem of this sort, we'd prefer to leak the item than suffer the other likely consequences. Per further analysis of bug #19527. Author: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/646741.1782157515@sss.pgh.pa.us Backpatch-through: 17 Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/ef01ca6dbca54e9bf3abea01c357b346847ebcf3 Modified Files -------------- src/backend/utils/resowner/resowner.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) ^ permalink raw reply [nested|flat] 7+ messages in thread
* pgsql: Fix unsafe order of operations in ResourceOwnerReleaseAll(). @ 2026-06-22 22:03 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 7+ messages in thread From: Tom Lane @ 2026-06-22 22:03 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix unsafe order of operations in ResourceOwnerReleaseAll(). This function called the resource-kind-specific ReleaseResource() method for each item before deleting that item from the resowner. That's backwards from the ordering in ResourceOwnerReleaseAllOfKind, and it's not very safe. If ReleaseResource throws an error then the subsequent abort cleanup will come back here and try to release that item again, possibly leading to a double-free or similar crash, and in any case risking an infinite error cleanup loop. This mistake explains why the pgcrypto bug just fixed in 80bb0ebcc led to a crash rather than something more benign. Remove the item from the resowner, then call ReleaseResource, matching the way things were done before b8bff07da. If there is a problem of this sort, we'd prefer to leak the item than suffer the other likely consequences. Per further analysis of bug #19527. Author: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/646741.1782157515@sss.pgh.pa.us Backpatch-through: 17 Branch ------ REL_18_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/ac6a58a700da1262d669e970f7ccba66f916c412 Modified Files -------------- src/backend/utils/resowner/resowner.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) ^ permalink raw reply [nested|flat] 7+ messages in thread
* pgsql: Fix unsafe order of operations in ResourceOwnerReleaseAll(). @ 2026-06-22 22:03 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 7+ messages in thread From: Tom Lane @ 2026-06-22 22:03 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix unsafe order of operations in ResourceOwnerReleaseAll(). This function called the resource-kind-specific ReleaseResource() method for each item before deleting that item from the resowner. That's backwards from the ordering in ResourceOwnerReleaseAllOfKind, and it's not very safe. If ReleaseResource throws an error then the subsequent abort cleanup will come back here and try to release that item again, possibly leading to a double-free or similar crash, and in any case risking an infinite error cleanup loop. This mistake explains why the pgcrypto bug just fixed in 80bb0ebcc led to a crash rather than something more benign. Remove the item from the resowner, then call ReleaseResource, matching the way things were done before b8bff07da. If there is a problem of this sort, we'd prefer to leak the item than suffer the other likely consequences. Per further analysis of bug #19527. Author: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/646741.1782157515@sss.pgh.pa.us Backpatch-through: 17 Branch ------ REL_17_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/011eedcdc3fe04000c46881333aaa0ac70a1aa1f Modified Files -------------- src/backend/utils/resowner/resowner.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) ^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2026-06-22 22:03 UTC | newest] Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2011-08-14 19:41 pgsql: Fix unsafe order of operations in foreign-table DDL commands. Tom Lane <tgl@sss.pgh.pa.us> 2011-08-14 19:41 pgsql: Fix unsafe order of operations in foreign-table DDL commands. Tom Lane <tgl@sss.pgh.pa.us> 2011-08-14 19:41 pgsql: Fix unsafe order of operations in foreign-table DDL commands. Tom Lane <tgl@sss.pgh.pa.us> 2011-08-14 19:41 pgsql: Fix unsafe order of operations in foreign-table DDL commands. Tom Lane <tgl@sss.pgh.pa.us> 2026-06-22 22:03 pgsql: Fix unsafe order of operations in ResourceOwnerReleaseAll(). Tom Lane <tgl@sss.pgh.pa.us> 2026-06-22 22:03 pgsql: Fix unsafe order of operations in ResourceOwnerReleaseAll(). Tom Lane <tgl@sss.pgh.pa.us> 2026-06-22 22:03 pgsql: Fix unsafe order of operations in ResourceOwnerReleaseAll(). Tom Lane <tgl@sss.pgh.pa.us>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox