While playing with xlogreader, I was lucky enough to see one of the many
record validations to fail. After having some fun with gdb, I found out that
in some cases the reader does not enforce enough data to be in state->readBuf
before copying into state->readRecordBuf starts. This should not happen if the
callback always reads XLOG_BLCKSZ bytes, but in fact only *reqLen* is the
mandatory size of the chunk delivered.

There are probably various ways to fix this problem. Attached is what I did in
my environment. I hit the problem on 9.4.1, but the patch seems to apply to
master too.

-- 
Antonin Houska
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt
Web: http://www.postgresql-support.de, http://www.cybertec.at

diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index 474137a..e6ebd9d 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -313,7 +313,21 @@ XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr, char **errormsg)
 		goto err;
 	}
 
-	len = XLOG_BLCKSZ - RecPtr % XLOG_BLCKSZ;
+	/* Bytes of the current record residing on the current page. */
+	len = Min(XLOG_BLCKSZ - targetRecOff, total_len);
+
+	/*
+	 * Nothing beyond the record header is guaranteed to be in state->readBuf
+	 * so far.
+	 */
+	if (readOff < targetRecOff + len)
+	{
+		readOff = ReadPageInternal(state, targetPagePtr, targetRecOff + len);
+
+		if (readOff < 0)
+			goto err;
+	}
+
 	if (total_len > len)
 	{
 		/* Need to reassemble record */
@@ -322,9 +336,11 @@ XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr, char **errormsg)
 		char	   *buffer;
 		uint32		gotlen;
 
+		Assert(readOff == targetRecOff + len);
+		Assert(readOff == XLOG_BLCKSZ);
+
 		/* Copy the first fragment of the record from the first page. */
-		memcpy(state->readRecordBuf,
-			   state->readBuf + RecPtr % XLOG_BLCKSZ, len);
+		memcpy(state->readRecordBuf, state->readBuf + targetRecOff, len);
 		buffer = state->readRecordBuf + len;
 		gotlen = len;
 
@@ -413,20 +429,16 @@ 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)
-			goto err;
+		Assert(readOff >= targetRecOff + len);
 
 		/* Record does not cross a page boundary */
 		if (!ValidXLogRecord(state, record, RecPtr))
 			goto err;
 
-		state->EndRecPtr = RecPtr + MAXALIGN(total_len);
+		state->EndRecPtr = RecPtr + MAXALIGN(len);
 
 		state->ReadRecPtr = RecPtr;
-		memcpy(state->readRecordBuf, record, total_len);
+		memcpy(state->readRecordBuf, record, len);
 	}
 
 	/*
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to