Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 80824dd

Browse files
committed
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
1 parent befd73c commit 80824dd

File tree

1 file changed

+33
-18
lines changed

1 file changed

+33
-18
lines changed

src/backend/nodes/tidbitmap.c

+33-18
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,10 @@ tbm_create(long maxbytes, dsa_area *dsa)
302302
tbm->maxentries = (int) nbuckets;
303303
tbm->lossify_start = 0;
304304
tbm->dsa = dsa;
305+
tbm->dsapagetable = InvalidDsaPointer;
306+
tbm->dsapagetableold = InvalidDsaPointer;
307+
tbm->ptpages = InvalidDsaPointer;
308+
tbm->ptchunks = InvalidDsaPointer;
305309

306310
return tbm;
307311
}
@@ -363,20 +367,23 @@ void
363367
tbm_free_shared_area(dsa_area *dsa, dsa_pointer dp)
364368
{
365369
TBMSharedIteratorState *istate = dsa_get_address(dsa, dp);
366-
PTEntryArray *ptbase = dsa_get_address(dsa, istate->pagetable);
370+
PTEntryArray *ptbase;
367371
PTIterationArray *ptpages;
368372
PTIterationArray *ptchunks;
369373

370-
if (pg_atomic_sub_fetch_u32(&ptbase->refcount, 1) == 0)
371-
dsa_free(dsa, istate->pagetable);
372-
373-
if (istate->spages)
374+
if (DsaPointerIsValid(istate->pagetable))
375+
{
376+
ptbase = dsa_get_address(dsa, istate->pagetable);
377+
if (pg_atomic_sub_fetch_u32(&ptbase->refcount, 1) == 0)
378+
dsa_free(dsa, istate->pagetable);
379+
}
380+
if (DsaPointerIsValid(istate->spages))
374381
{
375382
ptpages = dsa_get_address(dsa, istate->spages);
376383
if (pg_atomic_sub_fetch_u32(&ptpages->refcount, 1) == 0)
377384
dsa_free(dsa, istate->spages);
378385
}
379-
if (istate->schunks)
386+
if (DsaPointerIsValid(istate->schunks))
380387
{
381388
ptchunks = dsa_get_address(dsa, istate->schunks);
382389
if (pg_atomic_sub_fetch_u32(&ptchunks->refcount, 1) == 0)
@@ -786,7 +793,7 @@ tbm_prepare_shared_iterate(TIDBitmap *tbm)
786793
{
787794
dsa_pointer dp;
788795
TBMSharedIteratorState *istate;
789-
PTEntryArray *ptbase;
796+
PTEntryArray *ptbase = NULL;
790797
PTIterationArray *ptpages = NULL;
791798
PTIterationArray *ptchunks = NULL;
792799

@@ -797,7 +804,7 @@ tbm_prepare_shared_iterate(TIDBitmap *tbm)
797804
* Allocate TBMSharedIteratorState from DSA to hold the shared members and
798805
* lock, this will also be used by multiple worker for shared iterate.
799806
*/
800-
dp = dsa_allocate(tbm->dsa, sizeof(TBMSharedIteratorState));
807+
dp = dsa_allocate0(tbm->dsa, sizeof(TBMSharedIteratorState));
801808
istate = dsa_get_address(tbm->dsa, dp);
802809

803810
/*
@@ -856,7 +863,7 @@ tbm_prepare_shared_iterate(TIDBitmap *tbm)
856863
Assert(npages == tbm->npages);
857864
Assert(nchunks == tbm->nchunks);
858865
}
859-
else
866+
else if (tbm->status == TBM_ONE_PAGE)
860867
{
861868
/*
862869
* In one page mode allocate the space for one pagetable entry and
@@ -868,8 +875,8 @@ tbm_prepare_shared_iterate(TIDBitmap *tbm)
868875
ptpages->index[0] = 0;
869876
}
870877

871-
pg_atomic_init_u32(&ptbase->refcount, 0);
872-
878+
if (ptbase != NULL)
879+
pg_atomic_init_u32(&ptbase->refcount, 0);
873880
if (npages > 1)
874881
qsort_arg((void *) (ptpages->index), npages, sizeof(int),
875882
tbm_shared_comparator, (void *) ptbase->ptentry);
@@ -899,10 +906,11 @@ tbm_prepare_shared_iterate(TIDBitmap *tbm)
899906
* increase the refcount by 1 so that while freeing the shared iterator
900907
* we don't free pagetable and iterator array until its refcount becomes 0.
901908
*/
902-
pg_atomic_add_fetch_u32(&ptbase->refcount, 1);
903-
if (ptpages)
909+
if (ptbase != NULL)
910+
pg_atomic_add_fetch_u32(&ptbase->refcount, 1);
911+
if (ptpages != NULL)
904912
pg_atomic_add_fetch_u32(&ptpages->refcount, 1);
905-
if (ptchunks)
913+
if (ptchunks != NULL)
906914
pg_atomic_add_fetch_u32(&ptchunks->refcount, 1);
907915

908916
/* Initialize the iterator lock */
@@ -1069,9 +1077,16 @@ tbm_shared_iterate(TBMSharedIterator *iterator)
10691077
{
10701078
TBMIterateResult *output = &iterator->output;
10711079
TBMSharedIteratorState *istate = iterator->state;
1072-
PagetableEntry *ptbase = iterator->ptbase->ptentry;
1073-
int *idxpages = iterator->ptpages->index;
1074-
int *idxchunks = iterator->ptchunks->index;
1080+
PagetableEntry *ptbase = NULL;
1081+
int *idxpages = NULL;
1082+
int *idxchunks = NULL;
1083+
1084+
if (iterator->ptbase != NULL)
1085+
ptbase = iterator->ptbase->ptentry;
1086+
if (iterator->ptpages != NULL)
1087+
idxpages = iterator->ptpages->index;
1088+
if (iterator->ptchunks != NULL)
1089+
idxchunks = iterator->ptchunks->index;
10751090

10761091
/* Acquire the LWLock before accessing the shared members */
10771092
LWLockAcquire(&istate->lock, LW_EXCLUSIVE);
@@ -1480,7 +1495,7 @@ tbm_attach_shared_iterate(dsa_area *dsa, dsa_pointer dp)
14801495
* Create the TBMSharedIterator struct, with enough trailing space to
14811496
* serve the needs of the TBMIterateResult sub-struct.
14821497
*/
1483-
iterator = (TBMSharedIterator *) palloc(sizeof(TBMSharedIterator) +
1498+
iterator = (TBMSharedIterator *) palloc0(sizeof(TBMSharedIterator) +
14841499
MAX_TUPLES_PER_PAGE * sizeof(OffsetNumber));
14851500

14861501
istate = (TBMSharedIteratorState *) dsa_get_address(dsa, dp);

0 commit comments

Comments
 (0)