pg.ddx.io pgsql-committers@postgresql.org mailing list archivehelp / color / mirror / Atom feed
pgsql: Fix broken list-munging in ecpg's remove_variables(). 6+ messages / 1 participants [nested] [flat]
* pgsql: Fix broken list-munging in ecpg's remove_variables(). @ 2024-12-01 19:16 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 6+ messages in thread From: Tom Lane @ 2024-12-01 19:16 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix broken list-munging in ecpg's remove_variables(). The loops over cursor argument variables neglected to ever advance "prevvar". The code would accidentally do the right thing anyway when removing the first or second list entry, but if it had to remove the third or later entry then it would also remove all entries between there and the first entry. AFAICS this would only matter for cursors that reference out-of-scope variables, which is a weird Informix compatibility hack; between that and the lack of impact for short lists, it's not so surprising that nobody has complained. Nonetheless it's a pretty obvious bug. It would have been more obvious if these loops used a more standard coding style for chasing the linked lists --- this business with the "prev" pointer sometimes pointing at the current list entry is confusing and overcomplicated. So rather than just add a minimal band-aid, I chose to rewrite the loops in the same style we use elsewhere, where the "prev" pointer is NULL until we are dealing with a non-first entry and we save the "next" pointer at the top of the loop. (Two of the four loops touched here are not actually buggy, but it seems better to make them all look alike.) Coverity discovered this problem, but not until 2b41de4a5 added code to free no-longer-needed arguments structs. With that, the incorrect link updates are possibly touching freed memory, and it complained about that. Nonetheless the list corruption hazard is ancient, so back-patch to all supported branches. Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/2f696453d2b39fea800d5f7d8e5d3e1a2266de24 Modified Files -------------- src/interfaces/ecpg/preproc/variable.c | 62 ++++++++++++++++------------------ 1 file changed, 29 insertions(+), 33 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Fix broken list-munging in ecpg's remove_variables(). @ 2024-12-01 19:16 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 6+ messages in thread From: Tom Lane @ 2024-12-01 19:16 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix broken list-munging in ecpg's remove_variables(). The loops over cursor argument variables neglected to ever advance "prevvar". The code would accidentally do the right thing anyway when removing the first or second list entry, but if it had to remove the third or later entry then it would also remove all entries between there and the first entry. AFAICS this would only matter for cursors that reference out-of-scope variables, which is a weird Informix compatibility hack; between that and the lack of impact for short lists, it's not so surprising that nobody has complained. Nonetheless it's a pretty obvious bug. It would have been more obvious if these loops used a more standard coding style for chasing the linked lists --- this business with the "prev" pointer sometimes pointing at the current list entry is confusing and overcomplicated. So rather than just add a minimal band-aid, I chose to rewrite the loops in the same style we use elsewhere, where the "prev" pointer is NULL until we are dealing with a non-first entry and we save the "next" pointer at the top of the loop. (Two of the four loops touched here are not actually buggy, but it seems better to make them all look alike.) Coverity discovered this problem, but not until 2b41de4a5 added code to free no-longer-needed arguments structs. With that, the incorrect link updates are possibly touching freed memory, and it complained about that. Nonetheless the list corruption hazard is ancient, so back-patch to all supported branches. Branch ------ REL_17_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/a963abd549e9e2b518460ccf31ea3728214207c0 Modified Files -------------- src/interfaces/ecpg/preproc/variable.c | 67 +++++++++++++++++----------------- 1 file changed, 33 insertions(+), 34 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Fix broken list-munging in ecpg's remove_variables(). @ 2024-12-01 19:16 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 6+ messages in thread From: Tom Lane @ 2024-12-01 19:16 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix broken list-munging in ecpg's remove_variables(). The loops over cursor argument variables neglected to ever advance "prevvar". The code would accidentally do the right thing anyway when removing the first or second list entry, but if it had to remove the third or later entry then it would also remove all entries between there and the first entry. AFAICS this would only matter for cursors that reference out-of-scope variables, which is a weird Informix compatibility hack; between that and the lack of impact for short lists, it's not so surprising that nobody has complained. Nonetheless it's a pretty obvious bug. It would have been more obvious if these loops used a more standard coding style for chasing the linked lists --- this business with the "prev" pointer sometimes pointing at the current list entry is confusing and overcomplicated. So rather than just add a minimal band-aid, I chose to rewrite the loops in the same style we use elsewhere, where the "prev" pointer is NULL until we are dealing with a non-first entry and we save the "next" pointer at the top of the loop. (Two of the four loops touched here are not actually buggy, but it seems better to make them all look alike.) Coverity discovered this problem, but not until 2b41de4a5 added code to free no-longer-needed arguments structs. With that, the incorrect link updates are possibly touching freed memory, and it complained about that. Nonetheless the list corruption hazard is ancient, so back-patch to all supported branches. Branch ------ REL_16_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/cca34f68c6fa827728f16e563b16d90aa1679ab2 Modified Files -------------- src/interfaces/ecpg/preproc/variable.c | 67 +++++++++++++++++----------------- 1 file changed, 33 insertions(+), 34 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Fix broken list-munging in ecpg's remove_variables(). @ 2024-12-01 19:16 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 6+ messages in thread From: Tom Lane @ 2024-12-01 19:16 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix broken list-munging in ecpg's remove_variables(). The loops over cursor argument variables neglected to ever advance "prevvar". The code would accidentally do the right thing anyway when removing the first or second list entry, but if it had to remove the third or later entry then it would also remove all entries between there and the first entry. AFAICS this would only matter for cursors that reference out-of-scope variables, which is a weird Informix compatibility hack; between that and the lack of impact for short lists, it's not so surprising that nobody has complained. Nonetheless it's a pretty obvious bug. It would have been more obvious if these loops used a more standard coding style for chasing the linked lists --- this business with the "prev" pointer sometimes pointing at the current list entry is confusing and overcomplicated. So rather than just add a minimal band-aid, I chose to rewrite the loops in the same style we use elsewhere, where the "prev" pointer is NULL until we are dealing with a non-first entry and we save the "next" pointer at the top of the loop. (Two of the four loops touched here are not actually buggy, but it seems better to make them all look alike.) Coverity discovered this problem, but not until 2b41de4a5 added code to free no-longer-needed arguments structs. With that, the incorrect link updates are possibly touching freed memory, and it complained about that. Nonetheless the list corruption hazard is ancient, so back-patch to all supported branches. Branch ------ REL_15_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/60b47525c14b1f98927ae61acc033bb6ac80d84b Modified Files -------------- src/interfaces/ecpg/preproc/variable.c | 67 +++++++++++++++++----------------- 1 file changed, 33 insertions(+), 34 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Fix broken list-munging in ecpg's remove_variables(). @ 2024-12-01 19:16 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 6+ messages in thread From: Tom Lane @ 2024-12-01 19:16 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix broken list-munging in ecpg's remove_variables(). The loops over cursor argument variables neglected to ever advance "prevvar". The code would accidentally do the right thing anyway when removing the first or second list entry, but if it had to remove the third or later entry then it would also remove all entries between there and the first entry. AFAICS this would only matter for cursors that reference out-of-scope variables, which is a weird Informix compatibility hack; between that and the lack of impact for short lists, it's not so surprising that nobody has complained. Nonetheless it's a pretty obvious bug. It would have been more obvious if these loops used a more standard coding style for chasing the linked lists --- this business with the "prev" pointer sometimes pointing at the current list entry is confusing and overcomplicated. So rather than just add a minimal band-aid, I chose to rewrite the loops in the same style we use elsewhere, where the "prev" pointer is NULL until we are dealing with a non-first entry and we save the "next" pointer at the top of the loop. (Two of the four loops touched here are not actually buggy, but it seems better to make them all look alike.) Coverity discovered this problem, but not until 2b41de4a5 added code to free no-longer-needed arguments structs. With that, the incorrect link updates are possibly touching freed memory, and it complained about that. Nonetheless the list corruption hazard is ancient, so back-patch to all supported branches. Branch ------ REL_14_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/52c7a44e9518b3bbb930184526e0ae4eac412a7b Modified Files -------------- src/interfaces/ecpg/preproc/variable.c | 67 +++++++++++++++++----------------- 1 file changed, 33 insertions(+), 34 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Fix broken list-munging in ecpg's remove_variables(). @ 2024-12-01 19:16 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 6+ messages in thread From: Tom Lane @ 2024-12-01 19:16 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix broken list-munging in ecpg's remove_variables(). The loops over cursor argument variables neglected to ever advance "prevvar". The code would accidentally do the right thing anyway when removing the first or second list entry, but if it had to remove the third or later entry then it would also remove all entries between there and the first entry. AFAICS this would only matter for cursors that reference out-of-scope variables, which is a weird Informix compatibility hack; between that and the lack of impact for short lists, it's not so surprising that nobody has complained. Nonetheless it's a pretty obvious bug. It would have been more obvious if these loops used a more standard coding style for chasing the linked lists --- this business with the "prev" pointer sometimes pointing at the current list entry is confusing and overcomplicated. So rather than just add a minimal band-aid, I chose to rewrite the loops in the same style we use elsewhere, where the "prev" pointer is NULL until we are dealing with a non-first entry and we save the "next" pointer at the top of the loop. (Two of the four loops touched here are not actually buggy, but it seems better to make them all look alike.) Coverity discovered this problem, but not until 2b41de4a5 added code to free no-longer-needed arguments structs. With that, the incorrect link updates are possibly touching freed memory, and it complained about that. Nonetheless the list corruption hazard is ancient, so back-patch to all supported branches. Branch ------ REL_13_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/e359cbb846788c584767288497beaa788a0f1b3e Modified Files -------------- src/interfaces/ecpg/preproc/variable.c | 67 +++++++++++++++++----------------- 1 file changed, 33 insertions(+), 34 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2024-12-01 19:16 UTC | newest] Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2024-12-01 19:16 pgsql: Fix broken list-munging in ecpg's remove_variables(). Tom Lane <tgl@sss.pgh.pa.us> 2024-12-01 19:16 pgsql: Fix broken list-munging in ecpg's remove_variables(). Tom Lane <tgl@sss.pgh.pa.us> 2024-12-01 19:16 pgsql: Fix broken list-munging in ecpg's remove_variables(). Tom Lane <tgl@sss.pgh.pa.us> 2024-12-01 19:16 pgsql: Fix broken list-munging in ecpg's remove_variables(). Tom Lane <tgl@sss.pgh.pa.us> 2024-12-01 19:16 pgsql: Fix broken list-munging in ecpg's remove_variables(). Tom Lane <tgl@sss.pgh.pa.us> 2024-12-01 19:16 pgsql: Fix broken list-munging in ecpg's remove_variables(). Tom Lane <tgl@sss.pgh.pa.us>
This inbox is served by DDX for PostgreSQL; see mirroring instructions for how to clone and mirror all data and code used for this inbox