Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitd2a1d4b

Browse files
committed
Fix dereference of dangling pointer in GiST index buffering build.
gistBuildCallback tried to fetch the size of an index tuple thatmight have already been freed by gistProcessEmptyingQueue.While this seems to usually be harmless in production builds,in principle it could result in a SIGSEGV, or more likely a bogusvalue for indtuplesSize leading to poor page-split decisions laterin the build.The memory management here is confusing and could stand to berefactored, but for the moment it seems to be enough to fetchthe tuple size sooner. AFAICT the indtuples[Size] totals aren'tused in between these places; even if they were, the updatedvalues shouldn't be any worse to use. So just move theincrementing of the totals up.It's not very clear why our valgrind-using buildfarm animalshaven't noticed this problem, because the relevant code pathdoes seem to be exercised according to the code coverage report.I think the reason that we didn't fix this bug after the firstreport is that I'd wanted to try to understand that better.However, now that it's been re-discovered let's just be pragmaticand fix it already.Original report by Alexander Lakhin (bug #16329),later rediscovered by Egor Chindyaskin (bug #17874).Patch by Alexander Lakhin (commentary by Pavel Borisov and me).Back-patch to all supported branches.Discussion:https://postgr.es/m/16329-7a6aa9b6fa1118a1@postgresql.orgDiscussion:https://postgr.es/m/17874-63ca6c7ce42d2103@postgresql.org
1 parentaad9a2e commitd2a1d4b

File tree

1 file changed

+13
-4
lines changed

1 file changed

+13
-4
lines changed

‎src/backend/access/gist/gistbuild.c

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -475,6 +475,19 @@ gistBuildCallback(Relation index,
475475
itup=gistFormTuple(buildstate->giststate,index,values,isnull, true);
476476
itup->t_tid=htup->t_self;
477477

478+
/* Update tuple count and total size. */
479+
buildstate->indtuples+=1;
480+
buildstate->indtuplesSize+=IndexTupleSize(itup);
481+
482+
/*
483+
* XXX In buffering builds, the tempCxt is also reset down inside
484+
* gistProcessEmptyingQueue(). This is not great because it risks
485+
* confusion and possible use of dangling pointers (for example, itup
486+
* might be already freed when control returns here). It's generally
487+
* better that a memory context be "owned" by only one function. However,
488+
* currently this isn't causing issues so it doesn't seem worth the amount
489+
* of refactoring that would be needed to avoid it.
490+
*/
478491
if (buildstate->bufferingMode==GIST_BUFFERING_ACTIVE)
479492
{
480493
/* We have buffers, so use them. */
@@ -490,10 +503,6 @@ gistBuildCallback(Relation index,
490503
buildstate->giststate,buildstate->heaprel, true);
491504
}
492505

493-
/* Update tuple count and total size. */
494-
buildstate->indtuples+=1;
495-
buildstate->indtuplesSize+=IndexTupleSize(itup);
496-
497506
MemoryContextSwitchTo(oldCtx);
498507
MemoryContextReset(buildstate->giststate->tempCxt);
499508

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp