Hi, hackers!
I have already written about the problem of InvalidPath [0] appearing. I
investigated this and found an error in the add_path() function, when we
reject a path, we free up the memory of the path, but do not delete
various mentions of it (for example, in the ancestor of relation, as in
the example below).
Thus, we may face the problem of accessing the freed memory.
I demonstrated this below using gdb when I call a query after running a
test in test/regression/sql/create_misc.sql:
*Query:*
:-- That ALTER TABLE should have added TOAST tables.
SELECT relname, reltoastrelid <> 0 AS has_toast_table
FROM pg_class
WHERE oid::regclass IN ('a_star', 'c_star')
ORDER BY 1;
--UPDATE b_star*
-- SET a = text 'gazpacho'
-- WHERE aa > 4;
SELECT class, aa, a FROM a_star*;
*gdb:
*
0x00007ff3f7325fda in epoll_wait (epfd=5, events=0x55bf9ee75c38,
maxevents=1, timeout=-1) at ../sysdeps/unix/sysv/linux/epoll_wait.c:30
30 ../sysdeps/unix/sysv/linux/epoll_wait.c: No such file or directory.
(gdb) b /home/alena/postgrespro_3/src/backend/optimizer/util/pathnode.c:620
Breakpoint 1 at 0x55bf9cfe4c65: file pathnode.c, line 621.
(gdb) c
Continuing.
Breakpoint 1, add_path (parent_rel=0x55bf9ef7f5c0,
new_path=0x55bf9ef7f4e0) at pathnode.c:621
621 if (!IsA(new_path, IndexPath))
(gdb) n
622 pfree(new_path);
(gdb) n
624 }
(gdb) p *new_path
$1 = {type = T_Invalid, pathtype = T_Invalid, parent =
0x7f7f7f7f7f7f7f7f, pathtarget = 0x7f7f7f7f7f7f7f7f,
param_info = 0x7f7f7f7f7f7f7f7f, parallel_aware = 127, parallel_safe
= 127, parallel_workers = 2139062143,
rows = 1.3824172084878715e+306, startup_cost =
1.3824172084878715e+306, total_cost = 1.3824172084878715e+306,
pathkeys = 0x7f7f7f7f7f7f7f7f}
*(gdb) p new_path
$2 = (Path *) 0x55bf9ef7f4e0*
(gdb) p ((ProjectionPath *)((SortPath*)parent_rel->pathlist->elements
[0].ptr_value)->subpath)->path->parent->cheapest_startup_path
*$20 = (struct Path *) 0x55bf9ef7f4e0*
(gdb) p *((ProjectionPath *)((SortPath*)parent_rel->pathlist->elements
[0].ptr_value)->subpath)->path->parent->cheapest_startup_path
$17 = {type = T_Invalid, pathtype = T_Invalid, parent =
0x7f7f7f7f7f7f7f7f, pathtarget = 0x7f7f7f7f7f7f7f7f,
param_info = 0x7f7f7f7f7f7f7f7f, parallel_aware = 127, parallel_safe
= 127, parallel_workers = 2139062143,
rows = 1.3824172084878715e+306, startup_cost =
1.3824172084878715e+306, total_cost = 1.3824172084878715e+306,
pathkeys = 0x7f7f7f7f7f7f7f7f}
(gdb) p (Path*)(((ProjectionPath
*)((SortPath*)parent_rel->pathlist->elements
[0].ptr_value)->subpath)->path->parent->pathlist->elements[1].ptr_value)
*$21 = (Path *) 0x55bf9ef7f4e0*
(gdb) p *(Path*)(((ProjectionPath
*)((SortPath*)parent_rel->pathlist->elements
[0].ptr_value)->subpath)->path->parent->pathlist->elements[1].ptr_value)
$19 = {type = T_Invalid, pathtype = T_Invalid, parent =
0x7f7f7f7f7f7f7f7f, pathtarget = 0x7f7f7f7f7f7f7f7f,
param_info = 0x7f7f7f7f7f7f7f7f, parallel_aware = 127, parallel_safe
= 127, parallel_workers = 2139062143,
rows = 1.3824172084878715e+306, startup_cost =
1.3824172084878715e+306, total_cost = 1.3824172084878715e+306,
pathkeys = 0x7f7f7f7f7f7f7f7f}
(gdb)
The same problem may be in the add_partial_path() function.
Unfortunately, I have not yet been able to find a problematic query with
the described case, but I have prepared a patch to fix this problem.
What do you think?
0.
https://www.postgresql.org/message-id/flat/01aa76cc-93e1-4de9-ab34-b3a890bf7...@postgrespro.ru
--
Regards,
Alena Rybakina
Postgres Professional
From b9483800eb9d8dc9bafb114c602c63d5cf596ee7 Mon Sep 17 00:00:00 2001
From: Alena Rybakina <a.rybak...@postgrespro.ru>
Date: Mon, 30 Oct 2023 14:22:21 +0300
Subject: [PATCH] Fix reject released path: we shouldn't only clean path, but
we should delete all mentions ottherwice we will face the problem access to
released memory.
---
src/backend/optimizer/util/pathnode.c | 26 ++++++++++++++++++++++++--
1 file changed, 24 insertions(+), 2 deletions(-)
diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c
index 0b1d17b9d33..4df9d34c9f1 100644
--- a/src/backend/optimizer/util/pathnode.c
+++ b/src/backend/optimizer/util/pathnode.c
@@ -619,7 +619,19 @@ add_path(RelOptInfo *parent_rel, Path *new_path)
{
/* Reject and recycle the new path */
if (!IsA(new_path, IndexPath))
- pfree(new_path);
+ {
+ if(new_path == new_path->parent->cheapest_startup_path)
+ new_path->parent->cheapest_startup_path = NULL;
+ if(new_path == new_path->parent->cheapest_total_path)
+ new_path->parent->cheapest_total_path = NULL;
+ foreach(p1, new_path->parent->pathlist)
+ {
+ Path *path = (Path *) lfirst(p1);
+
+ if (path == new_path)
+ new_path->parent->pathlist = foreach_delete_current(new_path->parent->pathlist, p1);
+ }
+ }
}
}
@@ -849,7 +861,17 @@ add_partial_path(RelOptInfo *parent_rel, Path *new_path)
else
{
/* Reject and recycle the new path */
- pfree(new_path);
+ if(new_path == new_path->parent->cheapest_startup_path)
+ new_path->parent->cheapest_startup_path = NULL;
+ if(new_path == new_path->parent->cheapest_total_path)
+ new_path->parent->cheapest_total_path = NULL;
+ foreach(p1, new_path->parent->pathlist)
+ {
+ Path *path = (Path *) lfirst(p1);
+
+ if (path == new_path)
+ new_path->parent->pathlist = foreach_delete_current(new_path->parent->pathlist, p1);
+ }
}
}
--
2.34.1
