From 8dd18cce6ed63fe8f329db13731abaf1114318b6 Mon Sep 17 00:00:00 2001 From: Dimitri Fontaine Date: Fri, 10 Jul 2026 19:23:51 +0200 Subject: [PATCH 1/2] Fix buffer overflow in parseTimeLineHistory when timeline > 1024 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TimeLineHistory.history was a fixed-size array of 1024 entries. When a cluster's current timeline exceeded 1024, the TIMELINE_HISTORY replication command returns more lines than the array can hold. parseTimeLineHistory would then write a "tip" entry at history[1024] — one slot past the end — corrupting fields that follow ReplicationSource in LocalPostgresServer, including pgIsRunning, currentLSN, and pgKind. The keeper then logged "nodeKindToString: unknown node kind" and behaved incorrectly. Replace the fixed array with a dynamically-allocated pointer. The buffer is allocated on first use inside parseTimeLineHistory() and grown with realloc() when needed (doubling strategy), so it handles any number of timelines without a hard cap. Because ReplicationSource is embedded in the long-lived Keeper struct, the buffer is reused across calls and freed by the OS on exit — no explicit cleanup path is required. Consumer code (cli_do_misc.c, pgsql.c) uses history[index] pointer subscript syntax throughout, which is unchanged whether history is an array or a heap pointer — no consumer edits needed. --- src/bin/common/pgsql.c | 57 +++++++++++++++++++++++++++++++++++++++--- src/bin/common/pgsql.h | 13 ++++++++-- 2 files changed, 65 insertions(+), 5 deletions(-) diff --git a/src/bin/common/pgsql.c b/src/bin/common/pgsql.c index 4fe32a084..01adf79ca 100644 --- a/src/bin/common/pgsql.c +++ b/src/bin/common/pgsql.c @@ -3190,8 +3190,59 @@ parseTimeLineHistory(const char *filename, const char *content, system->timelines.count = 0; - TimeLineHistoryEntry *entry = - &(system->timelines.history[system->timelines.count]); + /* + * Ensure the history buffer exists and has room for at least lineCount + 1 + * entries (all parsed lines plus the tip entry written after the loop). + * We reuse the buffer across calls; realloc only when capacity is tight. + */ + int needed = lineCount + 1; + + if (system->timelines.history == NULL) + { + int cap = PG_AUTOCTL_TIMELINES_INITIAL_CAPACITY; + + while (cap < needed) + { + cap *= 2; + } + + system->timelines.history = + (TimeLineHistoryEntry *) calloc(cap, sizeof(TimeLineHistoryEntry)); + + if (system->timelines.history == NULL) + { + log_error(ALLOCATION_FAILED_ERROR); + free(historyLines); + return false; + } + + system->timelines.capacity = cap; + } + else if (system->timelines.capacity < needed) + { + int cap = system->timelines.capacity; + + while (cap < needed) + { + cap *= 2; + } + + TimeLineHistoryEntry *newHistory = + (TimeLineHistoryEntry *) realloc(system->timelines.history, + cap * sizeof(TimeLineHistoryEntry)); + + if (newHistory == NULL) + { + log_error(ALLOCATION_FAILED_ERROR); + free(historyLines); + return false; + } + + system->timelines.history = newHistory; + system->timelines.capacity = cap; + } + + TimeLineHistoryEntry *entry = &(system->timelines.history[0]); for (lineNumber = 0; lineNumber < lineCount; lineNumber++) { @@ -3271,7 +3322,7 @@ parseTimeLineHistory(const char *filename, const char *content, /* * Create one more entry for the "tip" of the timeline, which has no entry - * in the history file. + * in the history file. Capacity was pre-checked above to include this slot. */ entry->tli = system->timeline; entry->begin = prevend; diff --git a/src/bin/common/pgsql.h b/src/bin/common/pgsql.h index 978f6b172..2b51b2eda 100644 --- a/src/bin/common/pgsql.h +++ b/src/bin/common/pgsql.h @@ -193,7 +193,7 @@ typedef struct NodeAddressArray #define InvalidXLogRecPtr 0 #define XLogRecPtrIsInvalid(r) ((r) == InvalidXLogRecPtr) -#define PG_AUTOCTL_MAX_TIMELINES 1024 +#define PG_AUTOCTL_TIMELINES_INITIAL_CAPACITY 16 typedef struct TimeLineHistoryEntry { @@ -203,10 +203,19 @@ typedef struct TimeLineHistoryEntry } TimeLineHistoryEntry; +/* + * TimeLineHistory holds a dynamically-allocated array of timeline history + * entries. The history pointer starts as NULL and is allocated on first use + * inside parseTimeLineHistory(); subsequent calls reuse the same buffer, + * growing it with realloc() when needed. No explicit free is required because + * this struct is embedded in long-lived per-process structs (LocalPostgresServer + * → ReplicationSource → IdentifySystem) that live for the process lifetime. + */ typedef struct TimeLineHistory { int count; - TimeLineHistoryEntry history[PG_AUTOCTL_MAX_TIMELINES]; + int capacity; + TimeLineHistoryEntry *history; } TimeLineHistory; From 44f707931f86d9f3c46ffdaf57490f5faa8acdeb Mon Sep 17 00:00:00 2001 From: Dimitri Fontaine Date: Fri, 10 Jul 2026 19:33:09 +0200 Subject: [PATCH 2/2] Zero-fill expanded region after realloc in parseTimeLineHistory Timeline history cannot shrink (each new TLI's history file is the parent's verbatim plus one appended line, and timelines are strictly increasing), so stale entries above count are never reached by existing consumers. Defensively zero the newly allocated region after realloc so that a future consumer reading past count by mistake gets zeroes rather than garbage. --- src/bin/common/pgsql.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/bin/common/pgsql.c b/src/bin/common/pgsql.c index 01adf79ca..e981aff89 100644 --- a/src/bin/common/pgsql.c +++ b/src/bin/common/pgsql.c @@ -3238,6 +3238,9 @@ parseTimeLineHistory(const char *filename, const char *content, return false; } + memset(newHistory + system->timelines.capacity, + 0, + (cap - system->timelines.capacity) * sizeof(TimeLineHistoryEntry)); system->timelines.history = newHistory; system->timelines.capacity = cap; }