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

Commit a466219

Browse files
committed
Preserve memory context of VarStringSortSupport buffers.
When enlarging the work buffers of a VarStringSortSupport object, varstrfastcmp_locale was careful to keep them in the ssup_cxt memory context; but varstr_abbrev_convert just used palloc(). The latter creates a hazard that the buffers could be freed out from under the VarStringSortSupport object, resulting in stomping on whatever gets allocated in that memory later. In practice, because we only use this code for ICU collations (cf. 3df9c37), the problem is confined to use of ICU collations. I believe it may have been unreachable before the introduction of incremental sort, too, as traditional sorting usually just uses one context for the duration of the sort. We could fix this by making the broken stanzas in varstr_abbrev_convert match the non-broken ones in varstrfastcmp_locale. However, it seems like a better idea to dodge the issue altogether by replacing the pfree-and-allocate-anew coding with repalloc, which automatically preserves the chunk's memory context. This fix does add a few cycles because repalloc will copy the chunk's content, which the existing coding assumes is useless. However, we don't expect that these buffer enlargement operations are performance-critical. Besides that, it's far from obvious that copying the buffer contents isn't required, since these stanzas make no effort to mark the buffers invalid by resetting last_returned, cache_blob, etc. That seems to be safe upon examination, but it's fragile and could easily get broken in future, which wouldn't get revealed in testing with short-to-moderate-size strings. Per bug #17584 from James Inform. Whether or not the issue is reachable in the older branches, this code has been broken on its own terms from its introduction, so patch all the way back. Discussion: https://postgr.es/m/17584-95c79b4a7d771f44@postgresql.org
1 parent 213bd06 commit a466219

File tree

1 file changed

+6
-10
lines changed

1 file changed

+6
-10
lines changed

src/backend/utils/adt/varlena.c

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,8 @@ typedef struct
8080
char *buf1; /* 1st string, or abbreviation original string
8181
* buf */
8282
char *buf2; /* 2nd string, or abbreviation strxfrm() buf */
83-
int buflen1;
84-
int buflen2;
83+
int buflen1; /* Allocated length of buf1 */
84+
int buflen2; /* Allocated length of buf2 */
8585
int last_len1; /* Length of last buf1 string/strxfrm() input */
8686
int last_len2; /* Length of last buf2 string/strxfrm() blob */
8787
int last_returned; /* Last comparison result (cache) */
@@ -2330,15 +2330,13 @@ varstrfastcmp_locale(char *a1p, int len1, char *a2p, int len2, SortSupport ssup)
23302330

23312331
if (len1 >= sss->buflen1)
23322332
{
2333-
pfree(sss->buf1);
23342333
sss->buflen1 = Max(len1 + 1, Min(sss->buflen1 * 2, MaxAllocSize));
2335-
sss->buf1 = MemoryContextAlloc(ssup->ssup_cxt, sss->buflen1);
2334+
sss->buf1 = repalloc(sss->buf1, sss->buflen1);
23362335
}
23372336
if (len2 >= sss->buflen2)
23382337
{
2339-
pfree(sss->buf2);
23402338
sss->buflen2 = Max(len2 + 1, Min(sss->buflen2 * 2, MaxAllocSize));
2341-
sss->buf2 = MemoryContextAlloc(ssup->ssup_cxt, sss->buflen2);
2339+
sss->buf2 = repalloc(sss->buf2, sss->buflen2);
23422340
}
23432341

23442342
/*
@@ -2518,9 +2516,8 @@ varstr_abbrev_convert(Datum original, SortSupport ssup)
25182516
/* By convention, we use buffer 1 to store and NUL-terminate */
25192517
if (len >= sss->buflen1)
25202518
{
2521-
pfree(sss->buf1);
25222519
sss->buflen1 = Max(len + 1, Min(sss->buflen1 * 2, MaxAllocSize));
2523-
sss->buf1 = palloc(sss->buflen1);
2520+
sss->buf1 = repalloc(sss->buf1, sss->buflen1);
25242521
}
25252522

25262523
/* Might be able to reuse strxfrm() blob from last call */
@@ -2607,10 +2604,9 @@ varstr_abbrev_convert(Datum original, SortSupport ssup)
26072604
/*
26082605
* Grow buffer and retry.
26092606
*/
2610-
pfree(sss->buf2);
26112607
sss->buflen2 = Max(bsize + 1,
26122608
Min(sss->buflen2 * 2, MaxAllocSize));
2613-
sss->buf2 = palloc(sss->buflen2);
2609+
sss->buf2 = repalloc(sss->buf2, sss->buflen2);
26142610
}
26152611

26162612
/*

0 commit comments

Comments
 (0)