Subject: pg_ctl/miscinit: print "MyStartTime" as a long long instead of long to avoid 2038 problem.

From: Max Johnson <[email-masked]>
Date: Tue, 24 Sep 2024 19:33:24 +0000

Hello  hackers,

I have found an instance of a time overflow with the start time that is written in "postmaster.pid". On a 32-bit Linux system, if the system date is past 01/19/2038, when you start Postgres with `pg_ctl start -D {datadir} ...`,  the start time will have rolled back to approximately 1900. This is an instance of the "2038 problem". On my system, pg_ctl will not exit if the start time has overflowed.

This can be fixed by casting "MyStartTime" to a long long instead of just a long in "src/backend/utils/init/miscinit.c". Additionally, in "src/bin/pg_ctl/pg_ctl.c", when we read that value from the file, we should use "atoll()" instead of "atol()" to ensure we are reading it as a long long.
I have verified that this fixes the start time overflow on my 32-bit arm system. My glibc is compiled with 64-bit time_t.
Most systems running Postgres likely aren't 32-bit, but for embedded systems, this is important to ensure 2038 compatibility.

This is a fairly trivial patch, and I do not currently see any issues with using long long. I was told on IRC that a regression test is likely not necessary for this patch.
I look forward to hearing any feedback. This is my first open-source contribution!

Thank you,

Max Johnson

Embedded Linux Engineer I

​

NovaTech, LLC

13555 W. 107th Street | Lenexa, KS 66215 ​

O: 913.451.1880

M: 913.742.4580​

novatechautomation.com<http://www.novatechautomation.com/> | NovaTechLinkedIn<https://www.linkedin.com/company/565017>

NovaTech Automation is Net Zero committed. #KeepItCool<https://www.keepitcool.earth/>

Receipt of this email implies compliance with our terms and conditions<https://www.novatechautomation.com/email-terms-conditions>.

From 70ec929f971577e3a78ab32d15b631954286aaf7 Mon Sep 17 00:00:00 2001
From: Max Johnson <[email-masked]>
Date: Mon, 23 Sep 2024 13:41:43 -0500
Subject: [PATCH] pg_ctl and miscinit: change type of MyStartTime

The start time variable needs to be 64 bits wide on all platforms in order
to support dates past 01/19/2038. This patch fixes an overflow error on
the start time in "postmaster.pid", which can cause pg_ctl to hang.
Change the value of "MyStartTime" to long long.
Signed-off-by: Max Johnson <[email-masked]>
---
 src/backend/utils/init/miscinit.c | 4 ++--
 src/bin/pg_ctl/pg_ctl.c           | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c
index 1f0b3175ae..8669e95817 100644
--- a/src/backend/utils/init/miscinit.c
+++ b/src/backend/utils/init/miscinit.c
@@ -1205,10 +1205,10 @@ CreateLockFile(const char *filename, bool amPostmaster,
 	 * both datadir and socket lockfiles; although more stuff may get added to
 	 * the datadir lockfile later.
 	 */
-	snprintf(buffer, sizeof(buffer), "%d\n%s\n%ld\n%d\n%s\n",
+	snprintf(buffer, sizeof(buffer), "%d\n%s\n%lld\n%d\n%s\n",
 			 amPostmaster ? (int) my_pid : -((int) my_pid),
 			 DataDir,
-			 (long) MyStartTime,
+			 (long long) MyStartTime,
 			 PostPortNumber,
 			 socketDir);
 
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index f9e0ee4eee..e138c5b236 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -624,7 +624,7 @@ wait_for_postmaster_start(pgpid_t pm_pid, bool do_checkpoint)
 			 * Allow 2 seconds slop for possible cross-process clock skew.
 			 */
 			pmpid = atol(optlines[LOCK_FILE_LINE_PID - 1]);
-			pmstart = atol(optlines[LOCK_FILE_LINE_START_TIME - 1]);
+			pmstart = atoll(optlines[LOCK_FILE_LINE_START_TIME - 1]);
 			if (pmstart >= start_time - 2 &&
 #ifndef WIN32
 				pmpid == pm_pid
-- 
2.34.1

---

From: Nathan Bossart <[email-masked]>
Date: Tue, 24 Sep 2024 15:26:47 -0500

On Tue, Sep 24, 2024 at 07:33:24PM +0000, Max Johnson wrote:
> I have found an instance of a time overflow with the start time that is
> written in "postmaster.pid". On a 32-bit Linux system, if the system date
> is past 01/19/2038, when you start Postgres with `pg_ctl start -D
> {datadir} ...`,  the start time will have rolled back to approximately
> 1900. This is an instance of the "2038 problem". On my system, pg_ctl
> will not exit if the start time has overflowed.

Nice find.  I think this has been the case since the start time was added
to the lock files [0].

> +			 (long long) MyStartTime,

I think we should use INT64_FORMAT here.  That'll choose the right length
modifier for the platform.  And I don't think we need to cast MyStartTime,
since it's a pg_time_t (which is just an int64).

[0] https://postgr.es/c/30aeda4

-- 
nathan

---

From: Tom Lane <[email-masked]>
Date: Tue, 24 Sep 2024 16:44:41 -0400

> since it's a pg_time_t (which is just an int64).

Agreed.  However, a quick grep finds half a dozen other places that
are casting MyStartTime to long.  We should fix them all.

Also note that if any of the other places are using translatable
format strings, INT64_FORMAT is problematic in that context, and
"long long" is a better answer for them.

(I've not dug in the history, but I rather imagine that this is all
a hangover from MyStartTime having once been plain "time_t".)

			regards, tom lane

---

From: Nathan Bossart <[email-masked]>
Date: Tue, 24 Sep 2024 15:58:08 -0500

> are casting MyStartTime to long.  We should fix them all.

+1

> "long long" is a better answer for them.

At a glance, I'm not seeing any translatable format strings that involve
MyStartTime.  But that is good to know...

-- 
nathan

---

From: Max Johnson <[email-masked]>
Date: Wed, 25 Sep 2024 15:17:45 +0000

Hi there,

I have amended my patch to reflect the changes that were discussed and have verified on my system that it works the same as before. I have also fixed a typo and changed the name of the patch to more accurately reflect what it does now. Please let me know if there is anything else you'd like me to do.

Thanks again,

Max Johnson

​

NovaTech, LLC

O: 913.451.1880

M: 913.742.4580​

________________________________
From: Nathan Bossart <[email-masked]>
Sent: Tuesday, September 24, 2024 3:58 PM
To: Tom Lane <[email-masked]>
Cc: Max Johnson <[email-masked]>; [email-masked] <[email-masked]>
Subject: Re: pg_ctl/miscinit: print "MyStartTime" as a long long instead of long to avoid 2038 problem.

+1

--
nathan
From 031944d8045f13df474d307608a2246306b06f2e Mon Sep 17 00:00:00 2001
From: Max Johnson <[email-masked]>
Date: Wed, 25 Sep 2024 10:05:46 -0500
Subject: [PATCH] pg_ctl/miscinit: don't cast MyStartTime to long.

The start time variable needs to be 64 bits wide on all platforms in
order to support dates past 01/19/2038. This patch fixes an overflow
error on 32-bit systems with the start time in "postmaster.pid", which
causes pg_ctl to hang.
Don't cast MyStartTime to long.

index 1f0b3175ae..33f8f49fa2 100644
+	snprintf(buffer, sizeof(buffer), "%d\n%s\n"INT64_FORMAT"\n%d\n%s\n",
+			 MyStartTime,
 			 PostPortNumber,
 			 socketDir);

---

From: Nathan Bossart <[email-masked]>
Date: Wed, 25 Sep 2024 13:48:35 -0500

On Wed, Sep 25, 2024 at 03:17:45PM +0000, Max Johnson wrote:
> I have amended my patch to reflect the changes that were discussed and
> have verified on my system that it works the same as before. I have also
> fixed a typo and changed the name of the patch to more accurately reflect
> what it does now. Please let me know if there is anything else you'd like
> me to do.

Thanks!  I went through all the other uses of MyStartTime and fixed those
as needed, too.  Please let me know what you think.

-- 
nathan
diff --git a/contrib/postgres_fdw/option.c b/contrib/postgres_fdw/option.c
index 630b304338..da16fc78d4 100644
--- a/contrib/postgres_fdw/option.c
+++ b/contrib/postgres_fdw/option.c
@@ -522,7 +522,8 @@ process_pgfdw_appname(const char *appname)
 				appendStringInfoString(&buf, application_name);
 				break;
 			case 'c':
-				appendStringInfo(&buf, "%lx.%x", (long) (MyStartTime), MyProcPid);
+				appendStringInfo(&buf, INT64_HEX_FORMAT ".%x",
+								 MyStartTime, MyProcPid);
 				break;
 			case 'C':
 				appendStringInfoString(&buf, cluster_name);
diff --git a/src/backend/utils/error/csvlog.c b/src/backend/utils/error/csvlog.c
index 855e130a97..acdffb6d06 100644
--- a/src/backend/utils/error/csvlog.c
+++ b/src/backend/utils/error/csvlog.c
@@ -120,7 +120,7 @@ write_csvlog(ErrorData *edata)
 	appendStringInfoChar(&buf, ',');
 
 	/* session id */
-	appendStringInfo(&buf, "%lx.%x", (long) MyStartTime, MyProcPid);
+	appendStringInfo(&buf, INT64_HEX_FORMAT ".%x", MyStartTime, MyProcPid);
 	appendStringInfoChar(&buf, ',');
 
 	/* Line number */
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 5cbb5b5416..70e4c30df3 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -2944,12 +2944,13 @@ log_status_format(StringInfo buf, const char *format, ErrorData *edata)
 				{
 					char		strfbuf[128];
 
-					snprintf(strfbuf, sizeof(strfbuf) - 1, "%lx.%x",
-							 (long) (MyStartTime), MyProcPid);
+					snprintf(strfbuf, sizeof(strfbuf) - 1, INT64_HEX_FORMAT ".%x",
+							 MyStartTime, MyProcPid);
 					appendStringInfo(buf, "%*s", padding, strfbuf);
 				}
 				else
-					appendStringInfo(buf, "%lx.%x", (long) (MyStartTime), MyProcPid);
+					appendStringInfo(buf, INT64_HEX_FORMAT ".%x",
+									 MyStartTime, MyProcPid);
 				break;
 			case 'p':
 				if (padding != 0)
diff --git a/src/backend/utils/error/jsonlog.c b/src/backend/utils/error/jsonlog.c
index bd0124869d..492383a89e 100644
--- a/src/backend/utils/error/jsonlog.c
+++ b/src/backend/utils/error/jsonlog.c
@@ -168,8 +168,8 @@ write_jsonlog(ErrorData *edata)
 	}
 
 	/* Session id */
-	appendJSONKeyValueFmt(&buf, "session_id", true, "%lx.%x",
-						  (long) MyStartTime, MyProcPid);
+	appendJSONKeyValueFmt(&buf, "session_id", true, INT64_HEX_FORMAT ".%x",
+						  MyStartTime, MyProcPid);
 
 	/* Line number */
 	appendJSONKeyValueFmt(&buf, "line_num", false, "%ld", log_line_number);
diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c
index 537d92c0cf..ef60f41b8c 100644
--- a/src/backend/utils/init/miscinit.c
+++ b/src/backend/utils/init/miscinit.c
@@ -1372,10 +1372,10 @@ CreateLockFile(const char *filename, bool amPostmaster,
 	 * both datadir and socket lockfiles; although more stuff may get added to
 	 * the datadir lockfile later.
 	 */
-	snprintf(buffer, sizeof(buffer), "%d\n%s\n%ld\n%d\n%s\n",
+	snprintf(buffer, sizeof(buffer), "%d\n%s\n" INT64_FORMAT "\n%d\n%s\n",
 			 amPostmaster ? (int) my_pid : -((int) my_pid),
 			 DataDir,
-			 (long) MyStartTime,
+			 MyStartTime,
 			 PostPortNumber,
 			 socketDir);
 
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index e7e878c22f..d6bb2c3311 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -618,7 +618,7 @@ wait_for_postmaster_start(pid_t pm_pid, bool do_checkpoint)
 			 * Allow 2 seconds slop for possible cross-process clock skew.
 			 */
 			pmpid = atol(optlines[LOCK_FILE_LINE_PID - 1]);
-			pmstart = atol(optlines[LOCK_FILE_LINE_START_TIME - 1]);
+			pmstart = atoll(optlines[LOCK_FILE_LINE_START_TIME - 1]);
 			if (pmstart >= start_time - 2 &&
 #ifndef WIN32
 				pmpid == pm_pid
diff --git a/src/include/c.h b/src/include/c.h
index dc1841346c..400c527397 100644
--- a/src/include/c.h
+++ b/src/include/c.h
@@ -547,6 +547,8 @@ typedef unsigned long long int uint64;
 /* snprintf format strings to use for 64-bit integers */
 #define INT64_FORMAT "%" INT64_MODIFIER "d"
 #define UINT64_FORMAT "%" INT64_MODIFIER "u"
+#define INT64_HEX_FORMAT "%" INT64_MODIFIER "x"
+#define UINT64_HEX_FORMAT "%" INT64_MODIFIER "x"
 
 /*
  * 128-bit signed and unsigned integers
diff --git a/src/test/modules/test_radixtree/test_radixtree.c b/src/test/modules/test_radixtree/test_radixtree.c
index 1d9165a3a2..3e6863f660 100644
--- a/src/test/modules/test_radixtree/test_radixtree.c
+++ b/src/test/modules/test_radixtree/test_radixtree.c
@@ -23,8 +23,6 @@
 /* uncomment to use shared memory for the tree */
 /* #define TEST_SHARED_RT */
 
-#define UINT64_HEX_FORMAT "%" INT64_MODIFIER "X"
-
 /* Convenient macros to test results */
 #define EXPECT_TRUE(expr)	\
 	do { \