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

Commit6b67db1

Browse files
committed
Be more wary about 32-bit integer overflow in pg_stat_statements.
We've heard a couple of reports of people having trouble withmulti-gigabyte-sized query-texts files. It occurred to me that on32-bit platforms, there could be an issue with integer overflowof calculations associated with the total query text size.Address that with several changes:1. Limit pg_stat_statements.max to INT_MAX / 2 not INT_MAX.The hashtable code will bound it to that anyway unless "long"is 64 bits. We still need overflow guards on its use, butthis helps.2. Add a check to prevent extending the query-texts file tomore than MaxAllocHugeSize. If it got that big, qtext_load_filewould certainly fail, so there's not much point in allowing it.Without this, we'd need to consider whether extent, query_offset,and related variables shouldn't be off_t not size_t.3. Adjust the comparisons in need_gc_qtexts() to be done in 64-bitarithmetic on all platforms. It appears possible that under duressthose multiplications could overflow 32 bits, yielding a falseconclusion that we need to garbage-collect the texts file, whichcould lead to repeatedly garbage-collecting after every hash tableinsertion.Per report from Bruno da Silva. I'm not convinced that theseissues fully explain his problem; there may be some other bug that'scontributing to the query-texts file becoming so large in the firstplace. But it did get that big, so#2 is a reasonable defense,and#3 could explain the reported performance difficulties.(See also commit8bbe4cb, which addressed some related bugs.The second Discussion: link is the thread that led up to that.)This issue is old, and is primarily a problem for old platforms,so back-patch.Discussion:https://postgr.es/m/CAB+Nuk93fL1Q9eLOCotvLP07g7RAv4vbdrkm0cVQohDVMpAb9A@mail.gmail.comDiscussion:https://postgr.es/m/5601D354.5000703@BlueTreble.com
1 parent331f8b8 commit6b67db1

File tree

1 file changed

+22
-4
lines changed

1 file changed

+22
-4
lines changed

‎contrib/pg_stat_statements/pg_stat_statements.c

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -408,7 +408,7 @@ _PG_init(void)
408408
&pgss_max,
409409
5000,
410410
100,
411-
INT_MAX,
411+
INT_MAX /2,
412412
PGC_POSTMASTER,
413413
0,
414414
NULL,
@@ -2053,6 +2053,18 @@ qtext_store(const char *query, int query_len,
20532053

20542054
*query_offset=off;
20552055

2056+
/*
2057+
* Don't allow the file to grow larger than what qtext_load_file can
2058+
* (theoretically) handle. This has been seen to be reachable on 32-bit
2059+
* platforms.
2060+
*/
2061+
if (unlikely(query_len >=MaxAllocHugeSize-off))
2062+
{
2063+
errno=EFBIG;/* not quite right, but it'll do */
2064+
fd=-1;
2065+
gotoerror;
2066+
}
2067+
20562068
/* Now write the data into the successfully-reserved part of the file */
20572069
fd=OpenTransientFile(PGSS_TEXT_FILE,O_RDWR |O_CREAT |PG_BINARY);
20582070
if (fd<0)
@@ -2238,8 +2250,14 @@ need_gc_qtexts(void)
22382250
SpinLockRelease(&s->mutex);
22392251
}
22402252

2241-
/* Don't proceed if file does not exceed 512 bytes per possible entry */
2242-
if (extent<512*pgss_max)
2253+
/*
2254+
* Don't proceed if file does not exceed 512 bytes per possible entry.
2255+
*
2256+
* Here and in the next test, 32-bit machines have overflow hazards if
2257+
* pgss_max and/or mean_query_len are large. Force the multiplications
2258+
* and comparisons to be done in uint64 arithmetic to forestall trouble.
2259+
*/
2260+
if ((uint64)extent< (uint64)512*pgss_max)
22432261
return false;
22442262

22452263
/*
@@ -2249,7 +2267,7 @@ need_gc_qtexts(void)
22492267
* query length in order to prevent garbage collection from thrashing
22502268
* uselessly.
22512269
*/
2252-
if (extent<pgss->mean_query_len*pgss_max*2)
2270+
if ((uint64)extent< (uint64)pgss->mean_query_len*pgss_max*2)
22532271
return false;
22542272

22552273
return true;

0 commit comments

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