This is an automated email from the ASF dual-hosted git repository. maxyang pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/cloudberry.git
commit 92bee97d0b943e049c5eb900bf5e93c7e7623d06 Author: RMT <[email protected]> AuthorDate: Wed Sep 20 13:36:26 2023 +0800 io limit: fix double free (#16459) io limit: fix double free. In 'alterResgroupCallback', the io_limit pointer of 'caps' and 'oldCaps' maybe point to the same location, so there is a double free potentially. In 'alterResgroupCallback', the 'oldCaps' will be filled in 'GetResGroupCapabilities', and the assign it to 'caps' via: caps = oldCaps To resolve this problem, the code should free the oldCaps.io_limit, and set it to NIL, when the io_limit has not been altered. So, if the io_limit has not been altered, caps.io_limit = oldCaps.io_limit = NIL. If io_limit has been altered, caps.io_limit != oldCaps.io_limit. --- src/backend/commands/resgroupcmds.c | 10 +++++++++- src/test/isolation2/input/resgroup/resgroup_io_limit.source | 6 +++++- src/test/isolation2/output/resgroup/resgroup_io_limit.source | 11 ++++++++++- 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/src/backend/commands/resgroupcmds.c b/src/backend/commands/resgroupcmds.c index ce5ab6d584f..6ebe1ac243e 100644 --- a/src/backend/commands/resgroupcmds.c +++ b/src/backend/commands/resgroupcmds.c @@ -438,6 +438,12 @@ AlterResourceGroup(AlterResourceGroupStmt *stmt) /* Load current resource group capabilities */ GetResGroupCapabilities(pg_resgroupcapability_rel, groupid, &oldCaps); + /* If io_limit not been altered, reset io_limit field to NIL */ + if (limitType != RESGROUP_LIMIT_TYPE_IO_LIMIT && oldCaps.io_limit != NIL) + { + cgroupOpsRoutine->freeio(oldCaps.io_limit); + oldCaps.io_limit = NIL; + } caps = oldCaps; switch (limitType) @@ -479,6 +485,8 @@ AlterResourceGroup(AlterResourceGroupStmt *stmt) } validateCapabilities(pg_resgroupcapability_rel, groupid, &caps, false); + AssertImply(limitType != RESGROUP_LIMIT_TYPE_IO_LIMIT, caps.io_limit == NIL); + AssertImply(limitType == RESGROUP_LIMIT_TYPE_IO_LIMIT, caps.io_limit != NIL); /* cpuset & cpu_max_percent can not coexist. * if cpuset is active, then cpu_max_percent must set to CPU_RATE_LIMIT_DISABLED, @@ -1123,7 +1131,7 @@ alterResgroupCallback(XactEvent event, void *arg) if (callbackCtx->caps.io_limit != NIL) cgroupOpsRoutine->freeio(callbackCtx->caps.io_limit); - if (callbackCtx->caps.io_limit != NIL) + if (callbackCtx->oldCaps.io_limit != NIL) cgroupOpsRoutine->freeio(callbackCtx->oldCaps.io_limit); pfree(callbackCtx); diff --git a/src/test/isolation2/input/resgroup/resgroup_io_limit.source b/src/test/isolation2/input/resgroup/resgroup_io_limit.source index 8b0c4d69dc6..a09414f3f99 100644 --- a/src/test/isolation2/input/resgroup/resgroup_io_limit.source +++ b/src/test/isolation2/input/resgroup/resgroup_io_limit.source @@ -56,6 +56,8 @@ ALTER RESOURCE GROUP rg_test_group5 SET io_limit 'rg_io_limit_ts_1:rbps=1000,wbp SELECT check_cgroup_io_max('rg_test_group5', 'rg_io_limit_ts_1', 'rbps=1048576000 wbps=1048576000 riops=max wiops=max'); +ALTER RESOURCE GROUP rg_test_group5 SET concurrency 1; + SELECT check_clear_io_max('rg_test_group5'); -- fault injector @@ -67,11 +69,13 @@ SELECT groupid, groupname, cpuset FROM gp_toolkit.gp_resgroup_config WHERE group SELECT gp_inject_fault('create_resource_group_fail', 'reset', 1); --- start_ignore -- view +-- start_ignore SELECT * from gp_toolkit.gp_resgroup_iostats_per_host; -- end_ignore +SELECT count(*) > 0 as r from gp_toolkit.gp_resgroup_iostats_per_host; + -- clean DROP RESOURCE GROUP rg_test_group1; DROP RESOURCE GROUP rg_test_group2; diff --git a/src/test/isolation2/output/resgroup/resgroup_io_limit.source b/src/test/isolation2/output/resgroup/resgroup_io_limit.source index 114522afa75..3e7bd7f2134 100644 --- a/src/test/isolation2/output/resgroup/resgroup_io_limit.source +++ b/src/test/isolation2/output/resgroup/resgroup_io_limit.source @@ -107,6 +107,9 @@ SELECT check_cgroup_io_max('rg_test_group5', 'rg_io_limit_ts_1', 'rbps=104857600 t (1 row) +ALTER RESOURCE GROUP rg_test_group5 SET concurrency 1; +ALTER + SELECT check_clear_io_max('rg_test_group5'); check_clear_io_max -------------------- @@ -134,8 +137,8 @@ SELECT gp_inject_fault('create_resource_group_fail', 'reset', 1); Success: (1 row) --- start_ignore -- view +-- start_ignore SELECT * from gp_toolkit.gp_resgroup_iostats_per_host; rsgname | hostname | tablespace | rbps | wbps | riops | wiops ----------------+----------+------------------+------+------+-------+------- @@ -147,6 +150,12 @@ SELECT * from gp_toolkit.gp_resgroup_iostats_per_host; (5 rows) -- end_ignore +SELECT count(*) > 0 as r from gp_toolkit.gp_resgroup_iostats_per_host; + r +------ + t +(1 row) + -- clean DROP RESOURCE GROUP rg_test_group1; DROP --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
