Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Fix access-off-end-of-array in clog.c.
authorTom Lane <tgl@sss.pgh.pa.us>
Fri, 6 Oct 2017 16:20:13 +0000 (12:20 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Fri, 6 Oct 2017 16:20:13 +0000 (12:20 -0400)
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

src/backend/access/transam/clog.c

index 0a7e2b310f1fbcf9e675ddb55e7fd88cecf03b85..41778584ba87f84680d6ea019e3eedf35978ba4a 100644 (file)
@@ -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;
    }
 }