Fix access-off-end-of-array in clog.c.

tglsfdc · tglsfdc · commit 4c20ee5f27a2 · 2017-10-06T12:20:13.000-04:00
Sloppy loop coding in set_status_by_pages() resulted in fetching one array element more than it should from the subxids[] array. The odds of this resulting in SIGSEGV are pretty small, but we've certainly seen that happen with similar mistakes elsewhere. While at it, we can get rid of an extra TransactionIdToPage() calculation per loop. Per report from David Binderman. Back-patch to all supported branches, since this code is quite old. Discussion: https://postgr.es/m/HE1PR0802MB2331CBA919CBFFF0C465EB429C710@HE1PR0802MB2331.eurprd08.prod.outlook.com
diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c
@@ -227,21 +227,27 @@ set_status_by_pages(int nsubxids, TransactionId *subxids,
 	int			offset = 0;
 	int			i = 0;
 
+	Assert(nsubxids > 0);		/* else the pageno fetch above is unsafe */
+
 	while (i < nsubxids)
 	{
 		int			num_on_page = 0;
+		int			nextpageno;
 
-		while (TransactionIdToPage(subxids[i]) == pageno && i < nsubxids)
+		do
 		{
+			nextpageno = TransactionIdToPage(subxids[i]);
+			if (nextpageno != pageno)
+				break;
 			num_on_page++;
 			i++;
-		}
+		} while (i < nsubxids);
 
 		TransactionIdSetPageStatus(InvalidTransactionId,
 								   num_on_page, subxids + offset,
 								   status, lsn, pageno);
 		offset = i;
-		pageno = TransactionIdToPage(subxids[offset]);
+		pageno = nextpageno;
 	}
 }
 
