Potential null pointer dereference in postgres.c
Hi hackers,
When backporting 66e94448 to older versions it was forgotten to check
malloc() result. In 16+ versions guc_malloc() is used to allocate
memory and it checks if the result pointer is NULL, so there is no
need to check it after guc_malloc(). Versions before 16 have no
guc_malloc(), and malloc() is used instead, but we have to check if
return value is NULL.
Please find attached patch for REL_15_STABLE. This should be fixed in
older versions too.
Best regards,
Karina Litskevich
Postgres Professional: http://postgrespro.com/
Attachments:
v1-0001-Check-if-malloc-returned-NULL.patchtext/x-patch; charset=US-ASCII; name=v1-0001-Check-if-malloc-returned-NULL.patchDownload
From f33e9823d6413b88c7a42f6509e5c796fd0bab95 Mon Sep 17 00:00:00 2001
From: Karina Litskevich <litskevichkarina@gmail.com>
Date: Thu, 5 Dec 2024 19:31:27 +0300
Subject: [PATCH v1] Check if malloc returned NULL
---
src/backend/tcop/postgres.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index c8c687d6f5..a9864f895b 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -3674,6 +3674,12 @@ check_restrict_nonsystem_relation_kind(char **newval, void **extra, GucSource so
/* Save the flags in *extra, for use by the assign function */
*extra = malloc(sizeof(int));
+
+ if (*extra == NULL)
+ ereport(ERROR,
+ (errcode(ERRCODE_OUT_OF_MEMORY),
+ errmsg("out of memory")));
+
*((int *) *extra) = flags;
return true;
--
2.34.1
Karina Litskevich <litskevichkarina@gmail.com> writes:
When backporting 66e94448 to older versions it was forgotten to check
malloc() result. In 16+ versions guc_malloc() is used to allocate
memory and it checks if the result pointer is NULL, so there is no
need to check it after guc_malloc(). Versions before 16 have no
guc_malloc(), and malloc() is used instead, but we have to check if
return value is NULL.
Yup, you're right. Thanks for the report!
regards, tom lane
I'm glad you are bringing up this issue. By the way, there are two more
annoying places in postmaster.c for pg16 and older. See, strdup() also may
fail if insufficient memory available.
PFA patch for a REL_16_STABLE. It also applies to older versions.
--
Best regards,
Maxim Orlov.
Attachments:
v2-0001-Use-pstrdup-for-remote_host-and-remote_port-save-.patchapplication/octet-stream; name=v2-0001-Use-pstrdup-for-remote_host-and-remote_port-save-.patchDownload
From 9aac56b8ac8df6837fb6fc0f14bd7a76bd83a631 Mon Sep 17 00:00:00 2001
From: Maxim Orlov <orlovmg@gmail.com>
Date: Fri, 6 Dec 2024 18:46:09 +0300
Subject: [PATCH v2] Use pstrdup for remote_host and remote_port save in port
structure
---
src/backend/postmaster/postmaster.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index b42aae41fce..81f97113915 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -4343,8 +4343,8 @@ BackendInitialize(Port *port)
* Save remote_host and remote_port in port structure (after this, they
* will appear in log_line_prefix data for log messages).
*/
- port->remote_host = strdup(remote_host);
- port->remote_port = strdup(remote_port);
+ port->remote_host = pstrdup(remote_host);
+ port->remote_port = pstrdup(remote_port);
/* And now we can issue the Log_connections message, if wanted */
if (Log_connections)
--
2.47.0
I have to admit I was wrong with previous v2 patch. Sorry.
Apparently, the chances of committing this very low, but here is the
correct one.
--
Best regards,
Maxim Orlov.
Attachments:
v3-0001-Use-pstrdup-for-remote_host-and-remote_port-save-.patchapplication/octet-stream; name=v3-0001-Use-pstrdup-for-remote_host-and-remote_port-save-.patchDownload
From c2f65e027996622ad299a61b783c2cd6377f2ed2 Mon Sep 17 00:00:00 2001
From: Maxim Orlov <orlovmg@gmail.com>
Date: Fri, 6 Dec 2024 18:46:09 +0300
Subject: [PATCH v3] Use pstrdup for remote_host and remote_port save in port
structure
---
src/backend/postmaster/postmaster.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 110d05d13e..be31d01760 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -4342,8 +4342,8 @@ BackendInitialize(Port *port)
* Save remote_host and remote_port in port structure (after this, they
* will appear in log_line_prefix data for log messages).
*/
- port->remote_host = strdup(remote_host);
- port->remote_port = strdup(remote_port);
+ port->remote_host = MemoryContextStrdup(TopMemoryContext, remote_host);
+ port->remote_port = MemoryContextStrdup(TopMemoryContext, remote_port);
/* And now we can issue the Log_connections message, if wanted */
if (Log_connections)
@@ -4374,7 +4374,9 @@ BackendInitialize(Port *port)
ret == 0 &&
strspn(remote_host, "0123456789.") < strlen(remote_host) &&
strspn(remote_host, "0123456789ABCDEFabcdef:") < strlen(remote_host))
- port->remote_hostname = strdup(remote_host);
+ {
+ port->remote_hostname = MemoryContextStrdup(TopMemoryContext, remote_host);
+ }
/*
* Ready to begin client interaction. We will give up and _exit(1) after
--
2.47.0