From e9110429207558b8977a7ddc768240756a669bfd Mon Sep 17 00:00:00 2001 From: ChangAo Chen Date: Mon, 31 Aug 2026 21:45:05 +0800 Subject: [PATCH v6] Prevent overflow in WAIT FOR LSN timeout handling WAIT FOR LSN accepted timeout values up to INT64_MAX milliseconds, but converting such values to an absolute timestamp could overflow when multiplying milliseconds by 1000. Parse timeout values as int and use int throughout the waiting interface, limiting the valid range to 0..INT_MAX milliseconds. Zero continues to mean waiting indefinitely. --- doc/src/sgml/ref/wait_for.sgml | 2 ++ src/backend/access/transam/xlogwait.c | 2 +- src/backend/commands/repack_worker.c | 4 ++-- src/backend/commands/wait.c | 30 ++++++------------------- src/include/access/xlogwait.h | 2 +- src/test/recovery/t/049_wait_for_lsn.pl | 13 +++++++++++ 6 files changed, 26 insertions(+), 27 deletions(-) diff --git a/doc/src/sgml/ref/wait_for.sgml b/doc/src/sgml/ref/wait_for.sgml index 01dc2a84a1a..cae4cebbf1f 100644 --- a/doc/src/sgml/ref/wait_for.sgml +++ b/doc/src/sgml/ref/wait_for.sgml @@ -152,6 +152,8 @@ WAIT FOR LSN 'lsn' milliseconds. Also it might be given as string literal with integer number of milliseconds or a number with unit (see ). + The valid range is 0 .. INT_MAX milliseconds. + A value of zero means waiting indefinitely. diff --git a/src/backend/access/transam/xlogwait.c b/src/backend/access/transam/xlogwait.c index eee90e7f626..2ea8c24a74f 100644 --- a/src/backend/access/transam/xlogwait.c +++ b/src/backend/access/transam/xlogwait.c @@ -437,7 +437,7 @@ WaitLSNTypeRequiresRecovery(WaitLSNType t) * or replica got promoted before the target LSN reached. */ WaitLSNResult -WaitForLSN(WaitLSNType lsnType, XLogRecPtr targetLSN, int64 timeout) +WaitForLSN(WaitLSNType lsnType, XLogRecPtr targetLSN, int timeout) { XLogRecPtr currentLSN; WaitLSNProcInfo *procInfo; diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index af7e2a94764..a9870d9c8f2 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -447,7 +447,7 @@ decode_concurrent_changes(LogicalDecodingContext *ctx, if (record == NULL) { - int64 timeout = 0; + int timeout = 0; WaitLSNResult res; /* @@ -466,7 +466,7 @@ decode_concurrent_changes(LogicalDecodingContext *ctx, * should already have been flushed to disk. */ if (!XLogRecPtrIsValid(lsn_upto)) - timeout = 100L; + timeout = 100; res = WaitForLSN(WAIT_LSN_TYPE_PRIMARY_FLUSH, ctx->reader->EndRecPtr + 1, timeout); diff --git a/src/backend/commands/wait.c b/src/backend/commands/wait.c index 9ba4c75021e..23d6b593cb5 100644 --- a/src/backend/commands/wait.c +++ b/src/backend/commands/wait.c @@ -35,7 +35,7 @@ ExecWaitStmt(ParseState *pstate, WaitStmt *stmt, bool isTopLevel, DestReceiver *dest) { XLogRecPtr lsn; - int64 timeout = 0; + int timeout = 0; WaitLSNResult waitLSNResult; WaitLSNType lsnType = WAIT_LSN_TYPE_STANDBY_REPLAY; /* default */ bool throw = true; @@ -92,7 +92,6 @@ ExecWaitStmt(ParseState *pstate, WaitStmt *stmt, bool isTopLevel, { char *timeout_str; const char *hintmsg; - double dval; if (timeout_specified) errorConflictingDefElem(defel, pstate); @@ -100,33 +99,18 @@ ExecWaitStmt(ParseState *pstate, WaitStmt *stmt, bool isTopLevel, timeout_str = defGetString(defel); - if (!parse_real(timeout_str, &dval, GUC_UNIT_MS, &hintmsg)) - { + if (!parse_int(timeout_str, &timeout, GUC_UNIT_MS, &hintmsg)) ereport(ERROR, errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("invalid timeout value: \"%s\"", timeout_str), - hintmsg ? errhint("%s", _(hintmsg)) : 0); - } - - /* - * Get rid of any fractional part in the input. This is so we - * don't fail on just-out-of-range values that would round into - * range. - */ - dval = rint(dval); + hintmsg ? errhint("%s", _(hintmsg)) : 0, + parser_errposition(pstate, defel->location)); - /* Range check */ - if (unlikely(isnan(dval) || !FLOAT8_FITS_IN_INT64(dval))) - ereport(ERROR, - errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), - errmsg("timeout value is out of range")); - - if (dval < 0) + if (timeout < 0) ereport(ERROR, errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("timeout cannot be negative")); - - timeout = (int64) dval; + errmsg("timeout cannot be negative"), + parser_errposition(pstate, defel->location)); } else if (strcmp(defel->defname, "no_throw") == 0) { diff --git a/src/include/access/xlogwait.h b/src/include/access/xlogwait.h index 07157f220ea..2bf0263e9e2 100644 --- a/src/include/access/xlogwait.h +++ b/src/include/access/xlogwait.h @@ -104,6 +104,6 @@ extern XLogRecPtr GetCurrentLSNForWaitType(WaitLSNType lsnType); extern void WaitLSNWakeup(WaitLSNType lsnType, XLogRecPtr currentLSN); extern void WaitLSNCleanup(void); extern WaitLSNResult WaitForLSN(WaitLSNType lsnType, XLogRecPtr targetLSN, - int64 timeout); + int timeout); #endif /* XLOG_WAIT_H */ diff --git a/src/test/recovery/t/049_wait_for_lsn.pl b/src/test/recovery/t/049_wait_for_lsn.pl index cb7d4d461de..c8644e09af3 100644 --- a/src/test/recovery/t/049_wait_for_lsn.pl +++ b/src/test/recovery/t/049_wait_for_lsn.pl @@ -343,6 +343,13 @@ $node_standby->psql( stderr => \$stderr); ok($stderr =~ /timeout cannot be negative/, "get error for negative timeout"); +# Test out of range timeout +$node_standby->psql( + 'postgres', + "WAIT FOR LSN '${test_lsn}' WITH (timeout '2147483648ms');", + stderr => \$stderr); +ok($stderr =~ /invalid timeout value: "2147483648ms"/, "get error for out of range timeout"); + # Test unknown parameter with WITH clause $node_standby->psql( 'postgres', @@ -407,6 +414,12 @@ $output = $node_standby->safe_psql( ok($output eq "timeout", "WAIT FOR WITH clause returns correct timeout status"); +# Test maximum timeout +$output = $node_standby->safe_psql( + 'postgres', qq[ + WAIT FOR LSN '${lsn2}' WITH (timeout '2147483647ms', no_throw);]); +ok($output eq "success", "maximum timeout value is accepted"); + # Test WITH clause error case - invalid option $node_standby->psql( 'postgres', -- 2.53.0