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

Commit 8b0a5cf

Browse files
committed
pg_stat_statements: fetch stmt location/length before it disappears.
When executing a utility statement, we must fetch everything we need out of the PlannedStmt data structure before calling standard_ProcessUtility. In certain cases (possibly only ROLLBACK in extended query protocol), that data structure will get freed during command execution. The situation is probably often harmless in production builds, but in debug builds we intentionally overwrite the freed memory with garbage, leading to picking up garbage values of statement location and length, typically causing an assertion failure later in pg_stat_statements. In non-debug builds, if something did go wrong it would likely lead to storing garbage for the query string. Report and fix by zhaoqigui (with cosmetic adjustments by me). It's an old problem, so back-patch to all supported versions. Discussion: https://postgr.es/m/17663-a344fd0675f92128@postgresql.org Discussion: https://postgr.es/m/1667307420050.56657@hundsun.com
1 parent d2354b6 commit 8b0a5cf

File tree

1 file changed

+14
-2
lines changed

1 file changed

+14
-2
lines changed

contrib/pg_stat_statements/pg_stat_statements.c

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1085,6 +1085,8 @@ pgss_ProcessUtility(PlannedStmt *pstmt, const char *queryString,
10851085
{
10861086
Node *parsetree = pstmt->utilityStmt;
10871087
uint64 saved_queryId = pstmt->queryId;
1088+
int saved_stmt_location = pstmt->stmt_location;
1089+
int saved_stmt_len = pstmt->stmt_len;
10881090

10891091
/*
10901092
* Force utility statements to get queryId zero. We do this even in cases
@@ -1150,6 +1152,16 @@ pgss_ProcessUtility(PlannedStmt *pstmt, const char *queryString,
11501152
}
11511153
PG_END_TRY();
11521154

1155+
/*
1156+
* CAUTION: do not access the *pstmt data structure again below here.
1157+
* If it was a ROLLBACK or similar, that data structure may have been
1158+
* freed. We must copy everything we still need into local variables,
1159+
* which we did above.
1160+
*
1161+
* For the same reason, we can't risk restoring pstmt->queryId to its
1162+
* former value, which'd otherwise be a good idea.
1163+
*/
1164+
11531165
INSTR_TIME_SET_CURRENT(duration);
11541166
INSTR_TIME_SUBTRACT(duration, start);
11551167

@@ -1174,8 +1186,8 @@ pgss_ProcessUtility(PlannedStmt *pstmt, const char *queryString,
11741186

11751187
pgss_store(queryString,
11761188
saved_queryId,
1177-
pstmt->stmt_location,
1178-
pstmt->stmt_len,
1189+
saved_stmt_location,
1190+
saved_stmt_len,
11791191
PGSS_EXEC,
11801192
INSTR_TIME_GET_MILLISEC(duration),
11811193
rows,

0 commit comments

Comments
 (0)