pg.ddx.io pgsql-committers@postgresql.org mailing list archivehelp / color / mirror / Atom feed
pgsql: Remove race conditions between ECPGdebug() and ecpg_log(). 6+ messages / 1 participants [nested] [flat]
* pgsql: Remove race conditions between ECPGdebug() and ecpg_log(). @ 2024-05-23 19:52 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 6+ messages in thread From: Tom Lane @ 2024-05-23 19:52 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Remove race conditions between ECPGdebug() and ecpg_log(). Coverity complains that ECPGdebug is accessing debugstream without holding debug_mutex, which is a fair complaint: we should take debug_mutex while changing the settings ecpg_log looks at. In some branches it also complains about unlocked use of simple_debug. I think it's intentional and safe to have a quick unlocked check of simple_debug at the start of ecpg_log, since that early exit will always be taken in non-debug cases. But we should recheck simple_debug after acquiring the mutex. In the worst case, calling ECPGdebug concurrently with ecpg_log in another thread could result in a null-pointer dereference due to debugstream transiently being NULL while simple_debug isn't 0. This is largely hypothetical, since it's unlikely anybody uses ECPGdebug() at all in the field, and our own regression tests don't seem to be hitting the theoretical race conditions either. Still, if we're going to the trouble of having mutexes here, we ought to be using them in a way that's actually safe not just almost safe. Hence, back-patch to all supported branches. Branch ------ REL_16_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/78030a66d75b19a6428f30e31b12bc6059c1fff3 Modified Files -------------- src/interfaces/ecpg/ecpglib/misc.c | 41 ++++++++++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 11 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Remove race conditions between ECPGdebug() and ecpg_log(). @ 2024-05-23 19:52 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 6+ messages in thread From: Tom Lane @ 2024-05-23 19:52 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Remove race conditions between ECPGdebug() and ecpg_log(). Coverity complains that ECPGdebug is accessing debugstream without holding debug_mutex, which is a fair complaint: we should take debug_mutex while changing the settings ecpg_log looks at. In some branches it also complains about unlocked use of simple_debug. I think it's intentional and safe to have a quick unlocked check of simple_debug at the start of ecpg_log, since that early exit will always be taken in non-debug cases. But we should recheck simple_debug after acquiring the mutex. In the worst case, calling ECPGdebug concurrently with ecpg_log in another thread could result in a null-pointer dereference due to debugstream transiently being NULL while simple_debug isn't 0. This is largely hypothetical, since it's unlikely anybody uses ECPGdebug() at all in the field, and our own regression tests don't seem to be hitting the theoretical race conditions either. Still, if we're going to the trouble of having mutexes here, we ought to be using them in a way that's actually safe not just almost safe. Hence, back-patch to all supported branches. Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/0162a9bde267a0dc88f35a8d286514c059e1e0e2 Modified Files -------------- src/interfaces/ecpg/ecpglib/misc.c | 39 +++++++++++++++++++++++++++----------- 1 file changed, 28 insertions(+), 11 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Remove race conditions between ECPGdebug() and ecpg_log(). @ 2024-05-23 19:52 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 6+ messages in thread From: Tom Lane @ 2024-05-23 19:52 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Remove race conditions between ECPGdebug() and ecpg_log(). Coverity complains that ECPGdebug is accessing debugstream without holding debug_mutex, which is a fair complaint: we should take debug_mutex while changing the settings ecpg_log looks at. In some branches it also complains about unlocked use of simple_debug. I think it's intentional and safe to have a quick unlocked check of simple_debug at the start of ecpg_log, since that early exit will always be taken in non-debug cases. But we should recheck simple_debug after acquiring the mutex. In the worst case, calling ECPGdebug concurrently with ecpg_log in another thread could result in a null-pointer dereference due to debugstream transiently being NULL while simple_debug isn't 0. This is largely hypothetical, since it's unlikely anybody uses ECPGdebug() at all in the field, and our own regression tests don't seem to be hitting the theoretical race conditions either. Still, if we're going to the trouble of having mutexes here, we ought to be using them in a way that's actually safe not just almost safe. Hence, back-patch to all supported branches. Branch ------ REL_15_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/e892e72b3c8ce9221003cddca6b8a88e2c951cad Modified Files -------------- src/interfaces/ecpg/ecpglib/misc.c | 41 ++++++++++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 11 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Remove race conditions between ECPGdebug() and ecpg_log(). @ 2024-05-23 19:52 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 6+ messages in thread From: Tom Lane @ 2024-05-23 19:52 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Remove race conditions between ECPGdebug() and ecpg_log(). Coverity complains that ECPGdebug is accessing debugstream without holding debug_mutex, which is a fair complaint: we should take debug_mutex while changing the settings ecpg_log looks at. In some branches it also complains about unlocked use of simple_debug. I think it's intentional and safe to have a quick unlocked check of simple_debug at the start of ecpg_log, since that early exit will always be taken in non-debug cases. But we should recheck simple_debug after acquiring the mutex. In the worst case, calling ECPGdebug concurrently with ecpg_log in another thread could result in a null-pointer dereference due to debugstream transiently being NULL while simple_debug isn't 0. This is largely hypothetical, since it's unlikely anybody uses ECPGdebug() at all in the field, and our own regression tests don't seem to be hitting the theoretical race conditions either. Still, if we're going to the trouble of having mutexes here, we ought to be using them in a way that's actually safe not just almost safe. Hence, back-patch to all supported branches. Branch ------ REL_12_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/cd8f1c1207994c8b3d41c48a02fa40547ee48bea Modified Files -------------- src/interfaces/ecpg/ecpglib/misc.c | 41 ++++++++++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 11 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Remove race conditions between ECPGdebug() and ecpg_log(). @ 2024-05-23 19:52 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 6+ messages in thread From: Tom Lane @ 2024-05-23 19:52 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Remove race conditions between ECPGdebug() and ecpg_log(). Coverity complains that ECPGdebug is accessing debugstream without holding debug_mutex, which is a fair complaint: we should take debug_mutex while changing the settings ecpg_log looks at. In some branches it also complains about unlocked use of simple_debug. I think it's intentional and safe to have a quick unlocked check of simple_debug at the start of ecpg_log, since that early exit will always be taken in non-debug cases. But we should recheck simple_debug after acquiring the mutex. In the worst case, calling ECPGdebug concurrently with ecpg_log in another thread could result in a null-pointer dereference due to debugstream transiently being NULL while simple_debug isn't 0. This is largely hypothetical, since it's unlikely anybody uses ECPGdebug() at all in the field, and our own regression tests don't seem to be hitting the theoretical race conditions either. Still, if we're going to the trouble of having mutexes here, we ought to be using them in a way that's actually safe not just almost safe. Hence, back-patch to all supported branches. Branch ------ REL_13_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/2e062b65539ea9f85cf591ffbc4cd2db30bf5f51 Modified Files -------------- src/interfaces/ecpg/ecpglib/misc.c | 41 ++++++++++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 11 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Remove race conditions between ECPGdebug() and ecpg_log(). @ 2024-05-23 19:52 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 6+ messages in thread From: Tom Lane @ 2024-05-23 19:52 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Remove race conditions between ECPGdebug() and ecpg_log(). Coverity complains that ECPGdebug is accessing debugstream without holding debug_mutex, which is a fair complaint: we should take debug_mutex while changing the settings ecpg_log looks at. In some branches it also complains about unlocked use of simple_debug. I think it's intentional and safe to have a quick unlocked check of simple_debug at the start of ecpg_log, since that early exit will always be taken in non-debug cases. But we should recheck simple_debug after acquiring the mutex. In the worst case, calling ECPGdebug concurrently with ecpg_log in another thread could result in a null-pointer dereference due to debugstream transiently being NULL while simple_debug isn't 0. This is largely hypothetical, since it's unlikely anybody uses ECPGdebug() at all in the field, and our own regression tests don't seem to be hitting the theoretical race conditions either. Still, if we're going to the trouble of having mutexes here, we ought to be using them in a way that's actually safe not just almost safe. Hence, back-patch to all supported branches. Branch ------ REL_14_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/0795a35d13089c203f1b1ef523824c2fc22a6af9 Modified Files -------------- src/interfaces/ecpg/ecpglib/misc.c | 41 ++++++++++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 11 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2024-05-23 19:52 UTC | newest] Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2024-05-23 19:52 pgsql: Remove race conditions between ECPGdebug() and ecpg_log(). Tom Lane <tgl@sss.pgh.pa.us> 2024-05-23 19:52 pgsql: Remove race conditions between ECPGdebug() and ecpg_log(). Tom Lane <tgl@sss.pgh.pa.us> 2024-05-23 19:52 pgsql: Remove race conditions between ECPGdebug() and ecpg_log(). Tom Lane <tgl@sss.pgh.pa.us> 2024-05-23 19:52 pgsql: Remove race conditions between ECPGdebug() and ecpg_log(). Tom Lane <tgl@sss.pgh.pa.us> 2024-05-23 19:52 pgsql: Remove race conditions between ECPGdebug() and ecpg_log(). Tom Lane <tgl@sss.pgh.pa.us> 2024-05-23 19:52 pgsql: Remove race conditions between ECPGdebug() and ecpg_log(). 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