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.

Author: RMTT

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);

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;

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