Avoid access to uninitialized memory in shared tidbitmap iteration.

Primarily, this didn't work correctly when the tidbitmap ended up
empty.

Dilip Kumar, per a report from Emre Hasegeli

Discussion: http://postgr.es/m/CAFiTN-ujHFKb8WSLhK54rfqQT3r2yiPQOyeBrCDsA4p9Fwp_jw@mail.gmail.com

diff --git a/src/backend/nodes/tidbitmap.c b/src/backend/nodes/tidbitmap.c
index 8e915646d93a1a6a06875835033c121572de2424..ae7a913c12c77c0e6ca1b13c10011fe32572c45c 100644 (file)
--- a/src/backend/nodes/tidbitmap.c
+++ b/src/backend/nodes/tidbitmap.c
@@ -302,6 +302,10 @@ tbm_create(long maxbytes, dsa_area *dsa)
    tbm->maxentries = (int) nbuckets;
    tbm->lossify_start = 0;
    tbm->dsa = dsa;
+   tbm->dsapagetable = InvalidDsaPointer;
+   tbm->dsapagetableold = InvalidDsaPointer;
+   tbm->ptpages = InvalidDsaPointer;
+   tbm->ptchunks = InvalidDsaPointer;
 
    return tbm;
 }
@@ -363,20 +367,23 @@ void
 tbm_free_shared_area(dsa_area *dsa, dsa_pointer dp)
 {
    TBMSharedIteratorState *istate = dsa_get_address(dsa, dp);
-   PTEntryArray *ptbase = dsa_get_address(dsa, istate->pagetable);
+   PTEntryArray *ptbase;
    PTIterationArray *ptpages;
    PTIterationArray *ptchunks;
 
-   if (pg_atomic_sub_fetch_u32(&ptbase->refcount, 1) == 0)
-       dsa_free(dsa, istate->pagetable);
-
-   if (istate->spages)
+   if (DsaPointerIsValid(istate->pagetable))
+   {
+       ptbase = dsa_get_address(dsa, istate->pagetable);
+       if (pg_atomic_sub_fetch_u32(&ptbase->refcount, 1) == 0)
+           dsa_free(dsa, istate->pagetable);
+   }
+   if (DsaPointerIsValid(istate->spages))
    {
        ptpages = dsa_get_address(dsa, istate->spages);
        if (pg_atomic_sub_fetch_u32(&ptpages->refcount, 1) == 0)
            dsa_free(dsa, istate->spages);
    }
-   if (istate->schunks)
+   if (DsaPointerIsValid(istate->schunks))
    {
        ptchunks = dsa_get_address(dsa, istate->schunks);
        if (pg_atomic_sub_fetch_u32(&ptchunks->refcount, 1) == 0)
@@ -786,7 +793,7 @@ tbm_prepare_shared_iterate(TIDBitmap *tbm)
 {
    dsa_pointer dp;
    TBMSharedIteratorState *istate;
-   PTEntryArray *ptbase;
+   PTEntryArray *ptbase = NULL;
    PTIterationArray *ptpages = NULL;
    PTIterationArray *ptchunks = NULL;
 
@@ -797,7 +804,7 @@ tbm_prepare_shared_iterate(TIDBitmap *tbm)
     * Allocate TBMSharedIteratorState from DSA to hold the shared members and
     * lock, this will also be used by multiple worker for shared iterate.
     */
-   dp = dsa_allocate(tbm->dsa, sizeof(TBMSharedIteratorState));
+   dp = dsa_allocate0(tbm->dsa, sizeof(TBMSharedIteratorState));
    istate = dsa_get_address(tbm->dsa, dp);
 
    /*
@@ -856,7 +863,7 @@ tbm_prepare_shared_iterate(TIDBitmap *tbm)
            Assert(npages == tbm->npages);
            Assert(nchunks == tbm->nchunks);
        }
-       else
+       else if (tbm->status == TBM_ONE_PAGE)
        {
            /*
             * In one page mode allocate the space for one pagetable entry and
@@ -868,8 +875,8 @@ tbm_prepare_shared_iterate(TIDBitmap *tbm)
            ptpages->index[0] = 0;
        }
 
-       pg_atomic_init_u32(&ptbase->refcount, 0);
-
+       if (ptbase != NULL)
+           pg_atomic_init_u32(&ptbase->refcount, 0);
        if (npages > 1)
            qsort_arg((void *) (ptpages->index), npages, sizeof(int),
                      tbm_shared_comparator, (void *) ptbase->ptentry);
@@ -899,10 +906,11 @@ tbm_prepare_shared_iterate(TIDBitmap *tbm)
     * increase the refcount by 1 so that while freeing the shared iterator
     * we don't free pagetable and iterator array until its refcount becomes 0.
     */
-   pg_atomic_add_fetch_u32(&ptbase->refcount, 1);
-   if (ptpages)
+   if (ptbase != NULL)
+       pg_atomic_add_fetch_u32(&ptbase->refcount, 1);
+   if (ptpages != NULL)
        pg_atomic_add_fetch_u32(&ptpages->refcount, 1);
-   if (ptchunks)
+   if (ptchunks != NULL)
        pg_atomic_add_fetch_u32(&ptchunks->refcount, 1);
 
    /* Initialize the iterator lock */
@@ -1069,9 +1077,16 @@ tbm_shared_iterate(TBMSharedIterator *iterator)
 {
    TBMIterateResult *output = &iterator->output;
    TBMSharedIteratorState *istate = iterator->state;
-   PagetableEntry *ptbase = iterator->ptbase->ptentry;
-   int        *idxpages = iterator->ptpages->index;
-   int        *idxchunks = iterator->ptchunks->index;
+   PagetableEntry *ptbase = NULL;
+   int        *idxpages = NULL;
+   int        *idxchunks = NULL;
+
+   if (iterator->ptbase != NULL)
+       ptbase = iterator->ptbase->ptentry;
+   if (iterator->ptpages != NULL)
+       idxpages = iterator->ptpages->index;
+   if (iterator->ptchunks != NULL)
+       idxchunks = iterator->ptchunks->index;
 
    /* Acquire the LWLock before accessing the shared members */
    LWLockAcquire(&istate->lock, LW_EXCLUSIVE);
@@ -1480,7 +1495,7 @@ tbm_attach_shared_iterate(dsa_area *dsa, dsa_pointer dp)
     * Create the TBMSharedIterator struct, with enough trailing space to
     * serve the needs of the TBMIterateResult sub-struct.
     */
-   iterator = (TBMSharedIterator *) palloc(sizeof(TBMSharedIterator) +
+   iterator = (TBMSharedIterator *) palloc0(sizeof(TBMSharedIterator) +
                                 MAX_TUPLES_PER_PAGE * sizeof(OffsetNumber));
 
    istate = (TBMSharedIteratorState *) dsa_get_address(dsa, dp);