public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v11 1/4] Move callback-call from ReadPageInternal to XLogReadRecord.
10+ messages / 5 participants
[nested] [flat]
* [PATCH v11 1/4] Move callback-call from ReadPageInternal to XLogReadRecord.
@ 2019-09-05 11:21 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 10+ messages in thread
From: Kyotaro Horiguchi @ 2019-09-05 11:21 UTC (permalink / raw)
The current WAL record reader reads page data using a call back
function. Although it is not so problematic alone, it would be a
problem if we are going to do add tasks like encryption which is
performed on page data before WAL reader reads them. To avoid that the
record reader facility has to have a new code path corresponds to
every new callback, this patch separates page reader from WAL record
reading facility by modifying the current WAL record reader to a state
machine.
As the first step of that change, this patch moves the page reader
function out of ReadPageInternal, then the remaining tasks of the
function are taken over by the new function XLogNeedData. As the
result XLogPageRead directly calls the page reader callback function
according to the feedback from XLogNeedData.
---
src/backend/access/transam/xlog.c | 16 +-
src/backend/access/transam/xlogreader.c | 272 ++++++++++--------
src/backend/access/transam/xlogutils.c | 12 +-
.../replication/logical/logicalfuncs.c | 2 +-
src/backend/replication/walsender.c | 10 +-
src/bin/pg_rewind/parsexlog.c | 16 +-
src/bin/pg_waldump/pg_waldump.c | 14 +-
src/include/access/xlogreader.h | 23 +-
src/include/access/xlogutils.h | 2 +-
src/include/replication/logicalfuncs.h | 2 +-
10 files changed, 216 insertions(+), 153 deletions(-)
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 5f0ee50092..4a6bd6e002 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -884,7 +884,7 @@ static bool InstallXLogFileSegment(XLogSegNo *segno, char *tmppath,
static int XLogFileRead(XLogSegNo segno, int emode, TimeLineID tli,
int source, bool notfoundOk);
static int XLogFileReadAnyTLI(XLogSegNo segno, int emode, int source);
-static int XLogPageRead(XLogReaderState *xlogreader, XLogRecPtr targetPagePtr,
+static bool XLogPageRead(XLogReaderState *xlogreader, XLogRecPtr targetPagePtr,
int reqLen, XLogRecPtr targetRecPtr, char *readBuf);
static bool WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess,
bool fetching_ckpt, XLogRecPtr tliRecPtr);
@@ -4249,7 +4249,6 @@ ReadRecord(XLogReaderState *xlogreader, XLogRecPtr RecPtr, int emode,
XLogRecord *record;
XLogPageReadPrivate *private = (XLogPageReadPrivate *) xlogreader->private_data;
- /* Pass through parameters to XLogPageRead */
private->fetching_ckpt = fetching_ckpt;
private->emode = emode;
private->randAccess = (RecPtr != InvalidXLogRecPtr);
@@ -11537,7 +11536,7 @@ CancelBackup(void)
* XLogPageRead() to try fetching the record from another source, or to
* sleep and retry.
*/
-static int
+static bool
XLogPageRead(XLogReaderState *xlogreader, XLogRecPtr targetPagePtr, int reqLen,
XLogRecPtr targetRecPtr, char *readBuf)
{
@@ -11596,7 +11595,8 @@ retry:
readLen = 0;
readSource = 0;
- return -1;
+ xlogreader->readLen = -1;
+ return false;
}
}
@@ -11691,7 +11691,8 @@ retry:
goto next_record_is_invalid;
}
- return readLen;
+ xlogreader->readLen = readLen;
+ return true;
next_record_is_invalid:
lastSourceFailed = true;
@@ -11705,8 +11706,9 @@ next_record_is_invalid:
/* In standby-mode, keep trying */
if (StandbyMode)
goto retry;
- else
- return -1;
+
+ xlogreader->readLen = -1;
+ return false;
}
/*
diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index 67418b05f1..388662ade4 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -36,8 +36,8 @@
static void report_invalid_record(XLogReaderState *state, const char *fmt,...)
pg_attribute_printf(2, 3);
static bool allocate_recordbuf(XLogReaderState *state, uint32 reclength);
-static int ReadPageInternal(XLogReaderState *state, XLogRecPtr pageptr,
- int reqLen);
+static bool XLogNeedData(XLogReaderState *state, XLogRecPtr pageptr,
+ int reqLen, bool header_inclusive);
static void XLogReaderInvalReadState(XLogReaderState *state);
static bool ValidXLogRecordHeader(XLogReaderState *state, XLogRecPtr RecPtr,
XLogRecPtr PrevRecPtr, XLogRecord *record, bool randAccess);
@@ -245,7 +245,6 @@ XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr, char **errormsg)
uint32 targetRecOff;
uint32 pageHeaderSize;
bool gotheader;
- int readOff;
/*
* randAccess indicates whether to verify the previous-record pointer of
@@ -297,14 +296,20 @@ XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr, char **errormsg)
* byte to cover the whole record header, or at least the part of it that
* fits on the same page.
*/
- readOff = ReadPageInternal(state, targetPagePtr,
- Min(targetRecOff + SizeOfXLogRecord, XLOG_BLCKSZ));
- if (readOff < 0)
+ while (XLogNeedData(state, targetPagePtr,
+ Min(targetRecOff + SizeOfXLogRecord, XLOG_BLCKSZ),
+ targetRecOff != 0))
+ {
+ if (!state->read_page(state, state->readPagePtr, state->readLen,
+ RecPtr, state->readBuf))
+ break;
+ }
+
+ if (!state->page_verified)
goto err;
/*
- * ReadPageInternal always returns at least the page header, so we can
- * examine it now.
+ * We have at least the page header, so we can examine it now.
*/
pageHeaderSize = XLogPageHeaderSize((XLogPageHeader) state->readBuf);
if (targetRecOff == 0)
@@ -330,8 +335,8 @@ XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr, char **errormsg)
goto err;
}
- /* ReadPageInternal has verified the page header */
- Assert(pageHeaderSize <= readOff);
+ /* XLogNeedData has verified the page header */
+ Assert(pageHeaderSize <= state->readLen);
/*
* Read the record length.
@@ -404,18 +409,25 @@ XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr, char **errormsg)
do
{
+ int rest_len = total_len - gotlen;
+
/* Calculate pointer to beginning of next page */
targetPagePtr += XLOG_BLCKSZ;
/* Wait for the next page to become available */
- readOff = ReadPageInternal(state, targetPagePtr,
- Min(total_len - gotlen + SizeOfXLogShortPHD,
- XLOG_BLCKSZ));
+ while (XLogNeedData(state, targetPagePtr,
+ Min(rest_len, XLOG_BLCKSZ),
+ false))
+ {
+ if (!state->read_page(state, state->readPagePtr, state->readLen,
+ state->ReadRecPtr, state->readBuf))
+ break;
+ }
- if (readOff < 0)
+ if (!state->page_verified)
goto err;
- Assert(SizeOfXLogShortPHD <= readOff);
+ Assert(SizeOfXLogShortPHD <= state->readLen);
/* Check that the continuation on next page looks valid */
pageHeader = (XLogPageHeader) state->readBuf;
@@ -444,21 +456,14 @@ XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr, char **errormsg)
/* Append the continuation from this page to the buffer */
pageHeaderSize = XLogPageHeaderSize(pageHeader);
- if (readOff < pageHeaderSize)
- readOff = ReadPageInternal(state, targetPagePtr,
- pageHeaderSize);
-
- Assert(pageHeaderSize <= readOff);
+ Assert (pageHeaderSize <= state->readLen);
contdata = (char *) state->readBuf + pageHeaderSize;
len = XLOG_BLCKSZ - pageHeaderSize;
if (pageHeader->xlp_rem_len < len)
len = pageHeader->xlp_rem_len;
- if (readOff < pageHeaderSize + len)
- readOff = ReadPageInternal(state, targetPagePtr,
- pageHeaderSize + len);
-
+ Assert (pageHeaderSize + len <= state->readLen);
memcpy(buffer, (char *) contdata, len);
buffer += len;
gotlen += len;
@@ -488,9 +493,15 @@ XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr, char **errormsg)
else
{
/* Wait for the record data to become available */
- readOff = ReadPageInternal(state, targetPagePtr,
- Min(targetRecOff + total_len, XLOG_BLCKSZ));
- if (readOff < 0)
+ while (XLogNeedData(state, targetPagePtr,
+ Min(targetRecOff + total_len, XLOG_BLCKSZ), true))
+ {
+ if (!state->read_page(state, state->readPagePtr, state->readLen,
+ state->ReadRecPtr, state->readBuf))
+ break;
+ }
+
+ if (!state->page_verified)
goto err;
/* Record does not cross a page boundary */
@@ -533,109 +544,139 @@ err:
}
/*
- * Read a single xlog page including at least [pageptr, reqLen] of valid data
- * via the read_page() callback.
+ * Checks that an xlog page loaded in state->readBuf is including at least
+ * [pageptr, reqLen] and the page is valid. header_inclusive indicates that
+ * reqLen is calculated including page header length.
*
- * Returns -1 if the required page cannot be read for some reason; errormsg_buf
- * is set in that case (unless the error occurs in the read_page callback).
+ * Returns false if the buffer already contains the requested data, or found
+ * error. state->page_verified is set to true for the former and false for the
+ * latter.
*
- * We fetch the page from a reader-local cache if we know we have the required
- * data and if there hasn't been any error since caching the data.
+ * Otherwise returns true and requests data loaded onto state->readBuf by
+ * state->readPagePtr and state->readLen. The caller shall call this function
+ * again after filling the buffer at least with that portion of data and set
+ * state->readLen to the length of actually loaded data.
+ *
+ * If header_inclusive is false, corrects reqLen internally by adding the
+ * actual page header length and may request caller for new data.
*/
-static int
-ReadPageInternal(XLogReaderState *state, XLogRecPtr pageptr, int reqLen)
+static bool
+XLogNeedData(XLogReaderState *state, XLogRecPtr pageptr, int reqLen,
+ bool header_inclusive)
{
- int readLen;
uint32 targetPageOff;
XLogSegNo targetSegNo;
- XLogPageHeader hdr;
+ uint32 addLen = 0;
- Assert((pageptr % XLOG_BLCKSZ) == 0);
+ /* Some data is loaded, but page header is not verified yet. */
+ if (!state->page_verified &&
+ !XLogRecPtrIsInvalid(state->readPagePtr) && state->readLen >= 0)
+ {
+ uint32 pageHeaderSize;
- XLByteToSeg(pageptr, targetSegNo, state->segcxt.ws_segsize);
- targetPageOff = XLogSegmentOffset(pageptr, state->segcxt.ws_segsize);
+ /* just loaded new data so needs to verify page header */
- /* check whether we have all the requested data already */
- if (targetSegNo == state->seg.ws_segno &&
- targetPageOff == state->segoff && reqLen <= state->readLen)
- return state->readLen;
+ /* The caller must have loaded at least page header */
+ Assert (state->readLen >= SizeOfXLogShortPHD);
- /*
- * Data is not in our buffer.
- *
- * Every time we actually read the page, even if we looked at parts of it
- * before, we need to do verification as the read_page callback might now
- * be rereading data from a different source.
- *
- * Whenever switching to a new WAL segment, we read the first page of the
- * file and validate its header, even if that's not where the target
- * record is. This is so that we can check the additional identification
- * info that is present in the first page's "long" header.
- */
- if (targetSegNo != state->seg.ws_segno && targetPageOff != 0)
- {
- XLogRecPtr targetSegmentPtr = pageptr - targetPageOff;
+ /*
+ * We have enough data to check the header length. Recheck the loaded
+ * length against the actual header length.
+ */
+ pageHeaderSize = XLogPageHeaderSize((XLogPageHeader) state->readBuf);
- readLen = state->read_page(state, targetSegmentPtr, XLOG_BLCKSZ,
- state->currRecPtr,
- state->readBuf);
- if (readLen < 0)
- goto err;
+ /* Request more data if we don't have the full header. */
+ if (state->readLen < pageHeaderSize)
+ {
+ state->readLen = pageHeaderSize;
+ return true;
+ }
+
+ /* Now that we know we have the full header, validate it. */
+ if (!XLogReaderValidatePageHeader(state, state->readPagePtr,
+ (char *) state->readBuf))
+ {
+ /* That's bad. Force reading the page again. */
+ XLogReaderInvalReadState(state);
- /* we can be sure to have enough WAL available, we scrolled back */
- Assert(readLen == XLOG_BLCKSZ);
+ return false;
+ }
- if (!XLogReaderValidatePageHeader(state, targetSegmentPtr,
- state->readBuf))
- goto err;
+ state->page_verified = true;
+
+ XLByteToSeg(state->readPagePtr, state->seg.ws_segno,
+ state->segcxt.ws_segsize);
}
/*
- * First, read the requested data length, but at least a short page header
- * so that we can validate it.
+ * The loaded page may not be the one caller is supposing to read when we
+ * are verifying the first page of new segment. In that case, skip further
+ * verification and immediately load the target page.
*/
- readLen = state->read_page(state, pageptr, Max(reqLen, SizeOfXLogShortPHD),
- state->currRecPtr,
- state->readBuf);
- if (readLen < 0)
- goto err;
-
- Assert(readLen <= XLOG_BLCKSZ);
+ if (state->page_verified && pageptr == state->readPagePtr)
+ {
- /* Do we have enough data to check the header length? */
- if (readLen <= SizeOfXLogShortPHD)
- goto err;
+ /*
+ * calculate additional length for page header keeping the total
+ * length within the block size.
+ */
+ if (!header_inclusive)
+ {
+ uint32 pageHeaderSize =
+ XLogPageHeaderSize((XLogPageHeader) state->readBuf);
- Assert(readLen >= reqLen);
+ addLen = pageHeaderSize;
+ if (reqLen + pageHeaderSize <= XLOG_BLCKSZ)
+ addLen = pageHeaderSize;
+ else
+ addLen = XLOG_BLCKSZ - reqLen;
- hdr = (XLogPageHeader) state->readBuf;
+ Assert(addLen >= 0);
+ }
- /* still not enough */
- if (readLen < XLogPageHeaderSize(hdr))
- {
- readLen = state->read_page(state, pageptr, XLogPageHeaderSize(hdr),
- state->currRecPtr,
- state->readBuf);
- if (readLen < 0)
- goto err;
+ /* Return if we already have it. */
+ if (reqLen + addLen <= state->readLen)
+ return false;
}
+ /* Data is not in our buffer, request the caller for it. */
+ XLByteToSeg(pageptr, targetSegNo, state->segcxt.ws_segsize);
+ targetPageOff = XLogSegmentOffset(pageptr, state->segcxt.ws_segsize);
+ Assert((pageptr % XLOG_BLCKSZ) == 0);
+
/*
- * Now that we know we have the full header, validate it.
+ * Every time we request to load new data of a page to the caller, even if
+ * we looked at a part of it before, we need to do verification on the next
+ * invocation as the caller might now be rereading data from a different
+ * source.
*/
- if (!XLogReaderValidatePageHeader(state, pageptr, (char *) hdr))
- goto err;
-
- /* update read state information */
- state->seg.ws_segno = targetSegNo;
- state->segoff = targetPageOff;
- state->readLen = readLen;
+ state->page_verified = false;
- return readLen;
+ /*
+ * Whenever switching to a new WAL segment, we read the first page of the
+ * file and validate its header, even if that's not where the target
+ * record is. This is so that we can check the additional identification
+ * info that is present in the first page's "long" header.
+ * Don't do this if the caller requested the first page in the segment.
+ */
+ if (targetSegNo != state->seg.ws_segno && targetPageOff != 0)
+ {
+ /*
+ * Then we'll see that the targetSegNo now matches the ws_segno, and
+ * will not come back here, but will request the actual target page.
+ */
+ state->readPagePtr = pageptr - targetPageOff;
+ state->readLen = XLOG_BLCKSZ;
+ return true;
+ }
-err:
- XLogReaderInvalReadState(state);
- return -1;
+ /*
+ * Request the caller to load the page. We need at least a short page
+ * header so that we can validate it.
+ */
+ state->readPagePtr = pageptr;
+ state->readLen = Max(reqLen + addLen, SizeOfXLogShortPHD);
+ return true;
}
/*
@@ -644,9 +685,7 @@ err:
static void
XLogReaderInvalReadState(XLogReaderState *state)
{
- state->seg.ws_segno = 0;
- state->segoff = 0;
- state->readLen = 0;
+ state->readPagePtr = InvalidXLogRecPtr;
}
/*
@@ -924,7 +963,6 @@ XLogFindNextRecord(XLogReaderState *state, XLogRecPtr RecPtr)
XLogRecPtr targetPagePtr;
int targetRecOff;
uint32 pageHeaderSize;
- int readLen;
/*
* Compute targetRecOff. It should typically be equal or greater than
@@ -932,7 +970,7 @@ XLogFindNextRecord(XLogReaderState *state, XLogRecPtr RecPtr)
* that, except when caller has explicitly specified the offset that
* falls somewhere there or when we are skipping multi-page
* continuation record. It doesn't matter though because
- * ReadPageInternal() is prepared to handle that and will read at
+ * CheckPage() is prepared to handle that and will read at
* least short page-header worth of data
*/
targetRecOff = tmpRecPtr % XLOG_BLCKSZ;
@@ -940,19 +978,23 @@ XLogFindNextRecord(XLogReaderState *state, XLogRecPtr RecPtr)
/* scroll back to page boundary */
targetPagePtr = tmpRecPtr - targetRecOff;
- /* Read the page containing the record */
- readLen = ReadPageInternal(state, targetPagePtr, targetRecOff);
- if (readLen < 0)
+ while(XLogNeedData(state, targetPagePtr, targetRecOff,
+ targetRecOff != 0))
+ {
+ if (!state->read_page(state, state->readPagePtr, state->readLen,
+ state->ReadRecPtr, state->readBuf))
+ break;
+ }
+
+ if (!state->page_verified)
goto err;
header = (XLogPageHeader) state->readBuf;
pageHeaderSize = XLogPageHeaderSize(header);
- /* make sure we have enough data for the page header */
- readLen = ReadPageInternal(state, targetPagePtr, pageHeaderSize);
- if (readLen < 0)
- goto err;
+ /* we should have read the page header */
+ Assert (state->readLen >= pageHeaderSize);
/* skip over potential continuation data */
if (header->xlp_info & XLP_FIRST_IS_CONTRECORD)
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index 446760ed6e..060fbc0c4c 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -680,8 +680,8 @@ XLogTruncateRelation(RelFileNode rnode, ForkNumber forkNum,
void
XLogReadDetermineTimeline(XLogReaderState *state, XLogRecPtr wantPage, uint32 wantLength)
{
- const XLogRecPtr lastReadPage = (state->seg.ws_segno *
- state->segcxt.ws_segsize + state->segoff);
+ const XLogRecPtr lastReadPage = state->seg.ws_segno *
+ state->segcxt.ws_segsize + state->readLen;
Assert(wantPage != InvalidXLogRecPtr && wantPage % XLOG_BLCKSZ == 0);
Assert(wantLength <= XLOG_BLCKSZ);
@@ -813,7 +813,7 @@ wal_segment_open(XLogSegNo nextSegNo, WALSegmentContext *segcxt,
* exists for normal backends, so we have to do a check/sleep/repeat style of
* loop for now.
*/
-int
+bool
read_local_xlog_page(XLogReaderState *state, XLogRecPtr targetPagePtr,
int reqLen, XLogRecPtr targetRecPtr, char *cur_page)
{
@@ -915,7 +915,8 @@ read_local_xlog_page(XLogReaderState *state, XLogRecPtr targetPagePtr,
else if (targetPagePtr + reqLen > read_upto)
{
/* not enough data there */
- return -1;
+ state->readLen = -1;
+ return false;
}
else
{
@@ -933,7 +934,8 @@ read_local_xlog_page(XLogReaderState *state, XLogRecPtr targetPagePtr,
WALReadRaiseError(&errinfo);
/* number of valid bytes in the buffer */
- return count;
+ state->readLen = count;
+ return true;
}
/*
diff --git a/src/backend/replication/logical/logicalfuncs.c b/src/backend/replication/logical/logicalfuncs.c
index aa2106b152..4b0e8ea773 100644
--- a/src/backend/replication/logical/logicalfuncs.c
+++ b/src/backend/replication/logical/logicalfuncs.c
@@ -106,7 +106,7 @@ check_permissions(void)
(errmsg("must be superuser or replication role to use replication slots"))));
}
-int
+bool
logical_read_local_xlog_page(XLogReaderState *state, XLogRecPtr targetPagePtr,
int reqLen, XLogRecPtr targetRecPtr, char *cur_page)
{
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index ac9209747a..e2d64cc2e5 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -762,7 +762,7 @@ StartReplication(StartReplicationCmd *cmd)
* which has to do a plain sleep/busy loop, because the walsender's latch gets
* set every time WAL is flushed.
*/
-static int
+static bool
logical_read_xlog_page(XLogReaderState *state, XLogRecPtr targetPagePtr, int reqLen,
XLogRecPtr targetRecPtr, char *cur_page)
{
@@ -782,7 +782,10 @@ logical_read_xlog_page(XLogReaderState *state, XLogRecPtr targetPagePtr, int req
/* fail if not (implies we are going to shut down) */
if (flushptr < targetPagePtr + reqLen)
- return -1;
+ {
+ state->readLen = -1;
+ return false;
+ }
if (targetPagePtr + XLOG_BLCKSZ <= flushptr)
count = XLOG_BLCKSZ; /* more than one block available */
@@ -812,7 +815,8 @@ logical_read_xlog_page(XLogReaderState *state, XLogRecPtr targetPagePtr, int req
XLByteToSeg(targetPagePtr, segno, sendCxt->ws_segsize);
CheckXLogRemoved(segno, sendSeg->ws_tli);
- return count;
+ state->readLen = count;
+ return true;
}
/*
diff --git a/src/bin/pg_rewind/parsexlog.c b/src/bin/pg_rewind/parsexlog.c
index 1d03375a3e..795e84ff9f 100644
--- a/src/bin/pg_rewind/parsexlog.c
+++ b/src/bin/pg_rewind/parsexlog.c
@@ -44,7 +44,7 @@ typedef struct XLogPageReadPrivate
int tliIndex;
} XLogPageReadPrivate;
-static int SimpleXLogPageRead(XLogReaderState *xlogreader,
+static bool SimpleXLogPageRead(XLogReaderState *xlogreader,
XLogRecPtr targetPagePtr,
int reqLen, XLogRecPtr targetRecPtr, char *readBuf);
@@ -228,7 +228,7 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex,
}
/* XLogReader callback function, to read a WAL page */
-static int
+static bool
SimpleXLogPageRead(XLogReaderState *xlogreader, XLogRecPtr targetPagePtr,
int reqLen, XLogRecPtr targetRecPtr, char *readBuf)
{
@@ -283,7 +283,8 @@ SimpleXLogPageRead(XLogReaderState *xlogreader, XLogRecPtr targetPagePtr,
if (xlogreadfd < 0)
{
pg_log_error("could not open file \"%s\": %m", xlogfpath);
- return -1;
+ xlogreader->readLen = -1;
+ return false;
}
}
@@ -296,7 +297,8 @@ SimpleXLogPageRead(XLogReaderState *xlogreader, XLogRecPtr targetPagePtr,
if (lseek(xlogreadfd, (off_t) targetPageOff, SEEK_SET) < 0)
{
pg_log_error("could not seek in file \"%s\": %m", xlogfpath);
- return -1;
+ xlogreader->readLen = -1;
+ return false;
}
@@ -309,13 +311,15 @@ SimpleXLogPageRead(XLogReaderState *xlogreader, XLogRecPtr targetPagePtr,
pg_log_error("could not read file \"%s\": read %d of %zu",
xlogfpath, r, (Size) XLOG_BLCKSZ);
- return -1;
+ xlogreader->readLen = -1;
+ return false;
}
Assert(targetSegNo == xlogreadsegno);
xlogreader->seg.ws_tli = targetHistory[private->tliIndex].tli;
- return XLOG_BLCKSZ;
+ xlogreader->readLen = XLOG_BLCKSZ;
+ return true;
}
/*
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index 30a5851d87..9eece44626 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -324,9 +324,9 @@ WALDumpOpenSegment(XLogSegNo nextSegNo, WALSegmentContext *segcxt,
/*
* XLogReader read_page callback
*/
-static int
-WALDumpReadPage(XLogReaderState *state, XLogRecPtr targetPagePtr, int reqLen,
- XLogRecPtr targetPtr, char *readBuff)
+static bool
+XLogDumpReadPage(XLogReaderState *state, XLogRecPtr targetPagePtr, int reqLen,
+ XLogRecPtr targetPtr, char *readBuff)
{
XLogDumpPrivate *private = state->private_data;
int count = XLOG_BLCKSZ;
@@ -341,7 +341,8 @@ WALDumpReadPage(XLogReaderState *state, XLogRecPtr targetPagePtr, int reqLen,
else
{
private->endptr_reached = true;
- return -1;
+ state->readLen = -1;
+ return false;
}
}
@@ -366,7 +367,8 @@ WALDumpReadPage(XLogReaderState *state, XLogRecPtr targetPagePtr, int reqLen,
(Size) errinfo.wre_req);
}
- return count;
+ state->readLen = count;
+ return true;
}
/*
@@ -1027,7 +1029,7 @@ main(int argc, char **argv)
/* done with argument parsing, do the actual work */
/* we have everything we need, start reading */
- xlogreader_state = XLogReaderAllocate(WalSegSz, waldir, WALDumpReadPage,
+ xlogreader_state = XLogReaderAllocate(WalSegSz, waldir, XLogDumpReadPage,
&private);
if (!xlogreader_state)
fatal_error("out of memory");
diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h
index 0193611b7f..d990432ffe 100644
--- a/src/include/access/xlogreader.h
+++ b/src/include/access/xlogreader.h
@@ -49,7 +49,7 @@ typedef struct WALSegmentContext
typedef struct XLogReaderState XLogReaderState;
/* Function type definition for the read_page callback */
-typedef int (*XLogPageReadCB) (XLogReaderState *xlogreader,
+typedef bool (*XLogPageReadCB) (XLogReaderState *xlogreader,
XLogRecPtr targetPagePtr,
int reqLen,
XLogRecPtr targetRecPtr,
@@ -131,6 +131,20 @@ struct XLogReaderState
XLogRecPtr ReadRecPtr; /* start of last record read */
XLogRecPtr EndRecPtr; /* end+1 of last record read */
+ /* ----------------------------------------
+ * Communication with page reader
+ * readBuf is XLOG_BLCKSZ bytes, valid up to at least readLen bytes.
+ * ----------------------------------------
+ */
+ /* variables to communicate with page reader */
+ XLogRecPtr readPagePtr; /* page pointer to read */
+ int32 readLen; /* bytes requested to reader, or actual bytes
+ * read by reader, which must be larger than
+ * the request, or -1 on error */
+ TimeLineID readPageTLI; /* TLI for data currently in readBuf */
+ char *readBuf; /* buffer to store data */
+ bool page_verified; /* is the page on the buffer verified? */
+
/* ----------------------------------------
* Decoded representation of current record
@@ -157,13 +171,6 @@ struct XLogReaderState
* ----------------------------------------
*/
- /*
- * Buffer for currently read page (XLOG_BLCKSZ bytes, valid up to at least
- * readLen bytes)
- */
- char *readBuf;
- uint32 readLen;
-
/* last read XLOG position for data currently in readBuf */
WALSegmentContext segcxt;
WALOpenSegment seg;
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 0572b24192..0867ef0599 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -47,7 +47,7 @@ extern Buffer XLogReadBufferExtended(RelFileNode rnode, ForkNumber forknum,
extern Relation CreateFakeRelcacheEntry(RelFileNode rnode);
extern void FreeFakeRelcacheEntry(Relation fakerel);
-extern int read_local_xlog_page(XLogReaderState *state,
+extern bool read_local_xlog_page(XLogReaderState *state,
XLogRecPtr targetPagePtr, int reqLen,
XLogRecPtr targetRecPtr, char *cur_page);
diff --git a/src/include/replication/logicalfuncs.h b/src/include/replication/logicalfuncs.h
index 012096f183..54291221c3 100644
--- a/src/include/replication/logicalfuncs.h
+++ b/src/include/replication/logicalfuncs.h
@@ -11,7 +11,7 @@
#include "replication/logical.h"
-extern int logical_read_local_xlog_page(XLogReaderState *state,
+extern bool logical_read_local_xlog_page(XLogReaderState *state,
XLogRecPtr targetPagePtr,
int reqLen, XLogRecPtr targetRecPtr,
char *cur_page);
--
2.23.0
----Next_Part(Wed_Nov_27_12_09_23_2019_240)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="v11-0002-Move-page-reader-out-of-XLogReadRecord.patch"
^ permalink raw reply [nested|flat] 10+ messages in thread
* Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4)
@ 2024-12-15 02:50 Roberto C. Sánchez <[email protected]>
2024-12-30 21:50 ` Re: Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4) Roberto C. Sánchez <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Roberto C. Sánchez @ 2024-12-15 02:50 UTC (permalink / raw)
To: [email protected]
Greetings pgsql devs,
I would appreciate a review of my strategy for backporting the commits
related to CVE-2024-10978. (I am working with versions 11, 9.6, and 9.4,
for some older Debian releases.)
My conclusion is that of the two commits associated with CVE-2024-10978,
both are required in 11 and 9.6, but only one is required in 9.4.
I searched the list archives to see if there was a previous discussion
related to this and I found none. The only mention of CVE-2024-10978 was
the release announcement.
Details:
I started with the patches for version 12. These commits:
4c9d96f74ba4e7d01c086ca54f217e242dd65fae
c463338656ac47e5210fcf9fbf7d20efccce8de8
The log message for c463338656ac47e5210fcf9fbf7d20efccce8de8 includes
this bit:
----------
If cherry-picking the CVE-2024-10978 fixes, default to including this,
too. (This fixes an unintended side effect of fixing CVE-2024-10978.)
----------
Having spent a fair amount of time working on these patches, I have
successfully backported them to 11 and 9.6, but I encountered a problem
with 9.4. After analyzing the changes, I believe that the regression
which c463338656ac47e5210fcf9fbf7d20efccce8de8 is intended to address
does not affect 9.4. The basis for this belief is that after observing
the failure of the new setconfig regression test during the build of
9.4, I backed all of 4c9d96f74ba4e7d01c086ca54f217e242dd65fae and the
non-test portion of c463338656ac47e5210fcf9fbf7d20efccce8de8,
re-executed the build and observed the same failure.
The failure I observed occurs in the pg_upgrade test, under the heading
"Performing Upgrade" in the output:
Restoring global objects in the new cluster
*failure*
The end of ./build/contrib/pg_upgrade/pg_upgrade_utility.log has the
following:
ALTER ROLE "sbuild" WITH SUPERUSER INHERIT CREATEROLE CREATEDB LOGIN REPLICATION;
ALTER ROLE
ALTER ROLE "regress_authenticated_user_sr" SET "role" TO 'regress_current_user';
This is the same failure in both cases for 9.4 (both CVE-2024-10978
commits applied, and neither applied).
Is it then correct to conclude the behavior which regressed in 9.6 and
newer as a result of 4c9d96f74ba4e7d01c086ca54f217e242dd65fae was
introduced after 9.4? And that hence in the case of backporting to 9.4
that c463338656ac47e5210fcf9fbf7d20efccce8de8 can be left out?
Regards,
-Roberto
--
Roberto C. Sánchez
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4)
2024-12-15 02:50 Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4) Roberto C. Sánchez <[email protected]>
@ 2024-12-30 21:50 ` Roberto C. Sánchez <[email protected]>
2024-12-30 21:58 ` Re: Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4) Bruce Momjian <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Roberto C. Sánchez @ 2024-12-30 21:50 UTC (permalink / raw)
To: [email protected]
On Sat, Dec 14, 2024 at 09:50:23PM -0500, Roberto C. Sánchez wrote:
> Greetings pgsql devs,
>
> I would appreciate a review of my strategy for backporting the commits
> related to CVE-2024-10978. (I am working with versions 11, 9.6, and 9.4,
> for some older Debian releases.)
>
> My conclusion is that of the two commits associated with CVE-2024-10978,
> both are required in 11 and 9.6, but only one is required in 9.4.
>
I wonder if someone might be able to look at my original message and
help validate my analysis.
Regards,
-Roberto
--
Roberto C. Sánchez
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4)
2024-12-15 02:50 Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4) Roberto C. Sánchez <[email protected]>
2024-12-30 21:50 ` Re: Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4) Roberto C. Sánchez <[email protected]>
@ 2024-12-30 21:58 ` Bruce Momjian <[email protected]>
2024-12-31 01:23 ` Re: Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4) Michael Paquier <[email protected]>
2024-12-31 03:02 ` Re: Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4) Roberto C. Sánchez <[email protected]>
0 siblings, 2 replies; 10+ messages in thread
From: Bruce Momjian @ 2024-12-30 21:58 UTC (permalink / raw)
To: Roberto C. Sánchez <[email protected]>; +Cc: [email protected]
On Mon, Dec 30, 2024 at 04:50:12PM -0500, Roberto C. Sánchez wrote:
> On Sat, Dec 14, 2024 at 09:50:23PM -0500, Roberto C. Sánchez wrote:
> > Greetings pgsql devs,
> >
> > I would appreciate a review of my strategy for backporting the commits
> > related to CVE-2024-10978. (I am working with versions 11, 9.6, and 9.4,
> > for some older Debian releases.)
> >
> > My conclusion is that of the two commits associated with CVE-2024-10978,
> > both are required in 11 and 9.6, but only one is required in 9.4.
> >
> I wonder if someone might be able to look at my original message and
> help validate my analysis.
I saw your question and was kind of stumped about how to answer. We
rarely look at back branches for backpatch analysis, so I think we are
kind of confused on how to answer. Under what circumstances are you
supported versions of Postgres that we don't support? Is this part of
Debian policy? Is our five-year insufficient?
--
Bruce Momjian <[email protected]> https://momjian.us
EDB https://enterprisedb.com
Do not let urgent matters crowd out time for investment in the future.
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4)
2024-12-15 02:50 Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4) Roberto C. Sánchez <[email protected]>
2024-12-30 21:50 ` Re: Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4) Roberto C. Sánchez <[email protected]>
2024-12-30 21:58 ` Re: Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4) Bruce Momjian <[email protected]>
@ 2024-12-31 01:23 ` Michael Paquier <[email protected]>
2024-12-31 02:00 ` Re: Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4) Tom Lane <[email protected]>
2024-12-31 03:12 ` Re: Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4) Roberto C. Sánchez <[email protected]>
1 sibling, 2 replies; 10+ messages in thread
From: Michael Paquier @ 2024-12-31 01:23 UTC (permalink / raw)
To: Bruce Momjian <[email protected]>; +Cc: Roberto C. Sánchez <[email protected]>; [email protected]; Christoph Berg <[email protected]>
On Mon, Dec 30, 2024 at 04:58:26PM -0500, Bruce Momjian wrote:
> I saw your question and was kind of stumped about how to answer. We
> rarely look at back branches for backpatch analysis, so I think we are
> kind of confused on how to answer. Under what circumstances are you
> supported versions of Postgres that we don't support? Is this part of
> Debian policy?
So am I (I'd say that you are on your own for this one, still..).
It is the first time I hear about that on the lists, but perhaps
Christoph Berg would know better? Adding him in CC for comments.
Applying patches to older branches is a speciality in itself, and
requires a lot of work and analysis (not planning to do that here for
this specific CVE). The good thing is that 5a2fed911a85 has some
regression tests, so you could be more confident that what you are
doing is rather right. Now the code in this area has changed slightly
because of the introduction of parallel workers in 9.6, so that could
be tricky. I'd suggest to *not* bypass the work across multiple
branches at once as it can help in dealing with conflicts in a more
granular way, even if it may increase the analysis burden quite a bit.
While on it, note also 73c9f91a1b6d by the way, which is a follow up
of 5a2fed911a85 for CVE-2024-10978 related to parallel workers, it
would not apply to 9.4, for sure.
> Is our five-year insufficient?
FWIW, I'm already on the side that five-year support is quite good and
I'd side with not extending that, even argue about reducing it
(anti-tomato armor is now on). Backporting patches across up to 7
branches can be really tedious depending on what you are dealing with
in the backend.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4)
2024-12-15 02:50 Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4) Roberto C. Sánchez <[email protected]>
2024-12-30 21:50 ` Re: Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4) Roberto C. Sánchez <[email protected]>
2024-12-30 21:58 ` Re: Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4) Bruce Momjian <[email protected]>
2024-12-31 01:23 ` Re: Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4) Michael Paquier <[email protected]>
@ 2024-12-31 02:00 ` Tom Lane <[email protected]>
2024-12-31 03:27 ` Re: Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4) Roberto C. Sánchez <[email protected]>
1 sibling, 1 reply; 10+ messages in thread
From: Tom Lane @ 2024-12-31 02:00 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Bruce Momjian <[email protected]>; Roberto C. Sánchez <[email protected]>; [email protected]; Christoph Berg <[email protected]>
Michael Paquier <[email protected]> writes:
> On Mon, Dec 30, 2024 at 04:58:26PM -0500, Bruce Momjian wrote:
>> Is our five-year insufficient?
> FWIW, I'm already on the side that five-year support is quite good and
> I'd side with not extending that, even argue about reducing it
> (anti-tomato armor is now on). Backporting patches across up to 7
> branches can be really tedious depending on what you are dealing with
> in the backend.
Yeah, I can't see extending it, at least not under our current theory
of back-patching (nearly) every bug fix to all supported branches.
Could there be an intermediate state where older branches get only
"critical" fixes? (Security and data-loss bugs only, IMV.) Another
not-necessarily-exclusive idea is to designate only certain branches
as LTS. We could free up the developer bandwidth needed for LTS
by shortening the period in which non-LTS branches get full support.
This ties into a criticism I have of the criteria that outfits like
Debian and Red Hat seem to have for back-patching bug fixes: if it's
labeled "CVE" then it must get fixed, even for something as narrow
and low-impact as CVE-2024-10978. Meanwhile, even very critical
data-loss bugs are typically ignored. For a database, this verges
on insanity.
Maybe, if we were doing an only-critical-fixes LTS release series,
it'd be easier for downstream outfits to consume that instead of
cherry-picking security fixes. I'm just speculating though.
It's entirely possible that packagers would ignore our opinions
and keep on cherry-picking only security fixes, in which case
we'd be doing a lot of work for little return.
regards, tom lane
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4)
2024-12-15 02:50 Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4) Roberto C. Sánchez <[email protected]>
2024-12-30 21:50 ` Re: Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4) Roberto C. Sánchez <[email protected]>
2024-12-30 21:58 ` Re: Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4) Bruce Momjian <[email protected]>
2024-12-31 01:23 ` Re: Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4) Michael Paquier <[email protected]>
2024-12-31 02:00 ` Re: Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4) Tom Lane <[email protected]>
@ 2024-12-31 03:27 ` Roberto C. Sánchez <[email protected]>
0 siblings, 0 replies; 10+ messages in thread
From: Roberto C. Sánchez @ 2024-12-31 03:27 UTC (permalink / raw)
To: [email protected]
On Mon, Dec 30, 2024 at 09:00:59PM -0500, Tom Lane wrote:
>
> This ties into a criticism I have of the criteria that outfits like
> Debian and Red Hat seem to have for back-patching bug fixes: if it's
> labeled "CVE" then it must get fixed, even for something as narrow
> and low-impact as CVE-2024-10978.
I can't speak for RedHat, but on the Debian side we are actively pushing
back against this. It seems to be common practice for some corporate
outfits to manage compliance by running some commercially available
scanner and then trying to get every reported CVE "fixed" so that their
dashboard will be all green.
It's an uphill battle, though, since all-green dashboards trump
rationality more often than not in those discussions.
> Meanwhile, even very critical
> data-loss bugs are typically ignored. For a database, this verges
> on insanity.
>
Ack.
> Maybe, if we were doing an only-critical-fixes LTS release series,
> it'd be easier for downstream outfits to consume that instead of
> cherry-picking security fixes. I'm just speculating though.
> It's entirely possible that packagers would ignore our opinions
> and keep on cherry-picking only security fixes, in which case
> we'd be doing a lot of work for little return.
>
As far as Debian and PostgreSQL, we try to avoid cherry-picking and opt
for the point releases you publish whenever possible. Christoph already
does this for the newer releases (what is present in Debian stable and
sometimes LTS, depending on how long it has been since the Debian LTS
version has been released). For the older releases, however, we are
sometimes dealing with PostgreSQL versions that are no longer actively
developed upstream, so we have to resort to cherry-picking.
It is definitely not ideal, but the PostgreSQL code is very high quality
and we are generally able to handle the backporting without the need of
exernal assistance.
In any event, if releases of 11, 9.6, and 9.4 were still being published
to address new security issues (and data integrity bugs), then we would
simply be using those. However, given the age of those releases, it is
understandable that such releases are no longer being made.
Regards,
-Roberto
--
Roberto C. Sánchez
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4)
2024-12-15 02:50 Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4) Roberto C. Sánchez <[email protected]>
2024-12-30 21:50 ` Re: Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4) Roberto C. Sánchez <[email protected]>
2024-12-30 21:58 ` Re: Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4) Bruce Momjian <[email protected]>
2024-12-31 01:23 ` Re: Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4) Michael Paquier <[email protected]>
@ 2024-12-31 03:12 ` Roberto C. Sánchez <[email protected]>
1 sibling, 0 replies; 10+ messages in thread
From: Roberto C. Sánchez @ 2024-12-31 03:12 UTC (permalink / raw)
To: [email protected]
On Tue, Dec 31, 2024 at 10:23:29AM +0900, Michael Paquier wrote:
> On Mon, Dec 30, 2024 at 04:58:26PM -0500, Bruce Momjian wrote:
> > I saw your question and was kind of stumped about how to answer. We
> > rarely look at back branches for backpatch analysis, so I think we are
> > kind of confused on how to answer. Under what circumstances are you
> > supported versions of Postgres that we don't support? Is this part of
> > Debian policy?
>
> So am I (I'd say that you are on your own for this one, still..).
> It is the first time I hear about that on the lists, but perhaps
> Christoph Berg would know better? Adding him in CC for comments.
>
> Applying patches to older branches is a speciality in itself, and
> requires a lot of work and analysis (not planning to do that here for
> this specific CVE). The good thing is that 5a2fed911a85 has some
> regression tests, so you could be more confident that what you are
> doing is rather right. Now the code in this area has changed slightly
> because of the introduction of parallel workers in 9.6, so that could
> be tricky. I'd suggest to *not* bypass the work across multiple
> branches at once as it can help in dealing with conflicts in a more
> granular way, even if it may increase the analysis burden quite a bit.
>
Ack. I worked my way one branch at a time, specifically for the reason
you cited.
> While on it, note also 73c9f91a1b6d by the way, which is a follow up
> of 5a2fed911a85 for CVE-2024-10978 related to parallel workers, it
> would not apply to 9.4, for sure.
>
Definitely. That was relatively straightforward to figure out and
confirm.
Thanks for the hints.
Regards,
-Roberto
--
Roberto C. Sánchez
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4)
2024-12-15 02:50 Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4) Roberto C. Sánchez <[email protected]>
2024-12-30 21:50 ` Re: Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4) Roberto C. Sánchez <[email protected]>
2024-12-30 21:58 ` Re: Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4) Bruce Momjian <[email protected]>
@ 2024-12-31 03:02 ` Roberto C. Sánchez <[email protected]>
2024-12-31 03:25 ` Re: Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4) Bruce Momjian <[email protected]>
1 sibling, 1 reply; 10+ messages in thread
From: Roberto C. Sánchez @ 2024-12-31 03:02 UTC (permalink / raw)
To: [email protected]
Hi Bruce,
On Mon, Dec 30, 2024 at 04:58:26PM -0500, Bruce Momjian wrote:
> On Mon, Dec 30, 2024 at 04:50:12PM -0500, Roberto C. Sánchez wrote:
> > On Sat, Dec 14, 2024 at 09:50:23PM -0500, Roberto C. Sánchez wrote:
> > > Greetings pgsql devs,
> > >
> > > I would appreciate a review of my strategy for backporting the commits
> > > related to CVE-2024-10978. (I am working with versions 11, 9.6, and 9.4,
> > > for some older Debian releases.)
> > >
> > > My conclusion is that of the two commits associated with CVE-2024-10978,
> > > both are required in 11 and 9.6, but only one is required in 9.4.
> > >
> > I wonder if someone might be able to look at my original message and
> > help validate my analysis.
>
> I saw your question and was kind of stumped about how to answer. We
> rarely look at back branches for backpatch analysis, so I think we are
> kind of confused on how to answer. Under what circumstances are you
> supported versions of Postgres that we don't support? Is this part of
> Debian policy? Is our five-year insufficient?
>
Do you mean that branches for releases which are EOL are not looked at?
I understand that completely. What I was hoping for here was that
someone who was familiar with the old code might be able to look at my
analysis and either confirm that my conclusion is correct (the behavior
affected by the regression in the first commit was only introduced after
9.4) or not.
I did my best to structure my request in such a way that it would only
entail minimal effort to answer, assuming that it was viewed by someone
who had worked on those parts of the code that far back in the past.
As far as the five year support timeframe, that is amazing and much
more robust than many (most?) projects. Especially considering the size
and pace of development here. We do have a small (paid) team that tries
to support a specific subset of packages going back longer than 5 years.
If my request is not reasonable or somehow inappropriate, then please
consider it withdrawn.
Regards,
-Roberto
--
Roberto C. Sánchez
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4)
2024-12-15 02:50 Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4) Roberto C. Sánchez <[email protected]>
2024-12-30 21:50 ` Re: Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4) Roberto C. Sánchez <[email protected]>
2024-12-30 21:58 ` Re: Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4) Bruce Momjian <[email protected]>
2024-12-31 03:02 ` Re: Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4) Roberto C. Sánchez <[email protected]>
@ 2024-12-31 03:25 ` Bruce Momjian <[email protected]>
0 siblings, 0 replies; 10+ messages in thread
From: Bruce Momjian @ 2024-12-31 03:25 UTC (permalink / raw)
To: Roberto C. Sánchez <[email protected]>; +Cc: [email protected]
On Mon, Dec 30, 2024 at 10:02:18PM -0500, Roberto C. Sánchez wrote:
> Do you mean that branches for releases which are EOL are not looked at?
> I understand that completely. What I was hoping for here was that
> someone who was familiar with the old code might be able to look at my
> analysis and either confirm that my conclusion is correct (the behavior
> affected by the regression in the first commit was only introduced after
> 9.4) or not.
>
> I did my best to structure my request in such a way that it would only
> entail minimal effort to answer, assuming that it was viewed by someone
> who had worked on those parts of the code that far back in the past.
Agreed.
> As far as the five year support timeframe, that is amazing and much
> more robust than many (most?) projects. Especially considering the size
> and pace of development here. We do have a small (paid) team that tries
> to support a specific subset of packages going back longer than 5 years.
>
> If my request is not reasonable or somehow inappropriate, then please
> consider it withdrawn.
I think it is good you are asking --- I just don't know if anyone can
help.
--
Bruce Momjian <[email protected]> https://momjian.us
EDB https://enterprisedb.com
Do not let urgent matters crowd out time for investment in the future.
^ permalink raw reply [nested|flat] 10+ messages in thread
end of thread, other threads:[~2024-12-31 03:27 UTC | newest]
Thread overview: 10+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-09-05 11:21 [PATCH v11 1/4] Move callback-call from ReadPageInternal to XLogReadRecord. Kyotaro Horiguchi <[email protected]>
2024-12-15 02:50 Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4) Roberto C. Sánchez <[email protected]>
2024-12-30 21:50 ` Re: Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4) Roberto C. Sánchez <[email protected]>
2024-12-30 21:58 ` Re: Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4) Bruce Momjian <[email protected]>
2024-12-31 01:23 ` Re: Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4) Michael Paquier <[email protected]>
2024-12-31 02:00 ` Re: Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4) Tom Lane <[email protected]>
2024-12-31 03:27 ` Re: Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4) Roberto C. Sánchez <[email protected]>
2024-12-31 03:12 ` Re: Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4) Roberto C. Sánchez <[email protected]>
2024-12-31 03:02 ` Re: Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4) Roberto C. Sánchez <[email protected]>
2024-12-31 03:25 ` Re: Backport of CVE-2024-10978 fix to older pgsql versions (11, 9.6, and 9.4) Bruce Momjian <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox