On 25.06.24 16:21, Tom Lane wrote:
Peter Eisentraut <pe...@eisentraut.org> writes:
On 21.06.24 16:53, Tom Lane wrote:
Most of libpq gets at strerror_r via SOCK_STRERROR for Windows
portability. Is that relevant here?
Looking inside the OpenSSL code, it makes no efforts to translate
between winsock error codes and standard error codes, so I don't think
our workaround/replacement code needs to do that either.
Fair enough.
Btw., our source code comments say something like
"ERR_reason_error_string randomly refuses to map system errno values."
The reason it doesn't is exactly that it can't do it while maintaining
thread-safety.
Ah. Do you want to improve that comment while you're at it?
Here is a patch that fixes the strerror() call and updates the comments
a bit.
This ought to be backpatched like the original fix; ideally for the next
minor releases in about two weeks.
From fb61f5f897e6151957b6ea6e2dfd7905b48b7ca2 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Wed, 24 Jul 2024 14:58:12 +0200
Subject: [PATCH] libpq: Use strerror_r instead of strerror
Commit 453c4687377 introduced a use of strerror() into libpq, but that
is not thread-safe. Fix by using strerror_r() instead.
In passing, update some of the code comments added by 453c4687377, as
we have learned more about the reason for the change in OpenSSL that
started this.
Discussion: Discussion:
https://postgr.es/m/b6fb018b-f05c-4afd-abd3-318c649fa...@highgo.ca
---
src/backend/libpq/be-secure-openssl.c | 9 +++++----
src/interfaces/libpq/fe-secure-openssl.c | 11 ++++++-----
2 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/src/backend/libpq/be-secure-openssl.c
b/src/backend/libpq/be-secure-openssl.c
index 39b1a66236b..95b4d94a2e6 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -1453,10 +1453,11 @@ SSLerrmessage(unsigned long ecode)
return errreason;
/*
- * In OpenSSL 3.0.0 and later, ERR_reason_error_string randomly refuses
to
- * map system errno values. We can cover that shortcoming with this bit
- * of code. Older OpenSSL versions don't have the ERR_SYSTEM_ERROR
macro,
- * but that's okay because they don't have the shortcoming either.
+ * In OpenSSL 3.0.0 and later, ERR_reason_error_string does not map
system
+ * errno values anymore. (See OpenSSL source code for the explanation.)
+ * We can cover that shortcoming with this bit of code. Older OpenSSL
+ * versions don't have the ERR_SYSTEM_ERROR macro, but that's okay
because
+ * they don't have the shortcoming either.
*/
#ifdef ERR_SYSTEM_ERROR
if (ERR_SYSTEM_ERROR(ecode))
diff --git a/src/interfaces/libpq/fe-secure-openssl.c
b/src/interfaces/libpq/fe-secure-openssl.c
index 5c867106fb0..b6fffd7b9b0 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -1759,15 +1759,16 @@ SSLerrmessage(unsigned long ecode)
#endif
/*
- * In OpenSSL 3.0.0 and later, ERR_reason_error_string randomly refuses
to
- * map system errno values. We can cover that shortcoming with this bit
- * of code. Older OpenSSL versions don't have the ERR_SYSTEM_ERROR
macro,
- * but that's okay because they don't have the shortcoming either.
+ * In OpenSSL 3.0.0 and later, ERR_reason_error_string does not map
system
+ * errno values anymore. (See OpenSSL source code for the explanation.)
+ * We can cover that shortcoming with this bit of code. Older OpenSSL
+ * versions don't have the ERR_SYSTEM_ERROR macro, but that's okay
because
+ * they don't have the shortcoming either.
*/
#ifdef ERR_SYSTEM_ERROR
if (ERR_SYSTEM_ERROR(ecode))
{
- strlcpy(errbuf, strerror(ERR_GET_REASON(ecode)), SSL_ERR_LEN);
+ strerror_r(ERR_GET_REASON(ecode), errbuf, SSL_ERR_LEN);
return errbuf;
}
#endif
--
2.45.2
