Skip to content
This repository was archived by the owner on May 24, 2024. It is now read-only.

Commit 29a9f92

Browse files
authored
Fix getResUsage integer overflow.
In the function getResUsage it will set the group Id (type Oid)'s value from a string. Previous code use pg_atoi to do the value parse, this is not correct because type Oid is uint, we should use atooid. Another place is building the SQL involving group id it uses "%d", this commits fix this by using "%u".
1 parent d5573b9 commit 29a9f92

File tree

5 files changed

+70
-3
lines changed

5 files changed

+70
-3
lines changed

src/backend/access/transam/varsup.c

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#include "postmaster/autovacuum.h"
2525
#include "storage/pmsignal.h"
2626
#include "storage/proc.h"
27+
#include "utils/faultinjector.h"
2728
#include "utils/guc.h"
2829
#include "utils/syscache.h"
2930

@@ -601,6 +602,26 @@ GetNewObjectIdUnderLock(void)
601602
(ShmemVariableCache->nextOid)++;
602603
(ShmemVariableCache->oidCount)--;
603604

605+
#ifdef FAULT_INJECTOR
606+
if (SIMPLE_FAULT_INJECTOR("bump_oid") == FaultInjectorTypeSkip)
607+
{
608+
/*
609+
* CDB: we encounter high oid issues several times, we should
610+
* have some test-utils to verify logic under larger oid.
611+
*
612+
* NOTE: we do not have undo-bump, so take care when you decide to
613+
* use this fault inject. Currently, only resgroup test job uses it,
614+
* that is safe, becase resgroup job is an independent pipeline job.
615+
*/
616+
Oid large_oid = (1U<<31)+5; /* this value will overflow if taken as int32 */
617+
if (ShmemVariableCache->nextOid < large_oid)
618+
{
619+
ShmemVariableCache->nextOid = large_oid + 1;
620+
result = large_oid;
621+
}
622+
}
623+
#endif
624+
604625
return result;
605626
}
606627

src/backend/utils/resgroup/resgroup_helper.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ getResUsage(ResGroupStatCtx *ctx, Oid inGroupId)
105105
initStringInfo(&buffer);
106106
appendStringInfo(&buffer,
107107
"SELECT groupid, cpu_usage, memory_usage "
108-
"FROM pg_resgroup_get_status(%d)",
108+
"FROM pg_resgroup_get_status(%u)",
109109
inGroupId);
110110

111111
CdbDispatchCommand(buffer.data, DF_WITH_SNAPSHOT, &cdb_pgresults);
@@ -133,8 +133,7 @@ getResUsage(ResGroupStatCtx *ctx, Oid inGroupId)
133133
{
134134
const char *result;
135135
ResGroupStat *row = &ctx->groups[j];
136-
Oid groupId = pg_atoi(PQgetvalue(pg_result, j, 0),
137-
sizeof(Oid), 0);
136+
Oid groupId = atooid(PQgetvalue(pg_result, j, 0));
138137

139138
Assert(groupId == row->groupId);
140139

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
-- Test resgroup oid larger than int32.
2+
select gp_inject_fault('bump_oid', 'skip', dbid) from gp_segment_configuration where role = 'p' and content = -1;
3+
gp_inject_fault
4+
-----------------
5+
Success:
6+
(1 row)
7+
8+
create resource group rg_large_oid with (cpu_rate_limit=20, memory_limit=10);
9+
CREATE
10+
11+
select gp_inject_fault('bump_oid', 'reset', dbid) from gp_segment_configuration where role = 'p' and content = -1;
12+
gp_inject_fault
13+
-----------------
14+
Success:
15+
(1 row)
16+
17+
select max(oid)::bigint > (power(2,31) + 1)::bigint from pg_resgroup;
18+
?column?
19+
----------
20+
t
21+
(1 row)
22+
23+
-- count(*) > 0 to run the SQL but do not display the result
24+
select count(*) > 0 from pg_resgroup_get_status(NULL);
25+
?column?
26+
----------
27+
t
28+
(1 row)
29+
30+
drop resource group rg_large_oid;
31+
DROP

src/test/isolation2/isolation2_resgroup_schedule

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,4 +51,7 @@ test: resgroup/resgroup_functions
5151
# dump info
5252
test: resgroup/resgroup_dumpinfo
5353

54+
# test larget group id
55+
test: resgroup/resgroup_large_group_id
56+
5457
test: resgroup/disable_resgroup
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
-- Test resgroup oid larger than int32.
2+
select gp_inject_fault('bump_oid', 'skip', dbid) from gp_segment_configuration where role = 'p' and content = -1;
3+
4+
create resource group rg_large_oid with (cpu_rate_limit=20, memory_limit=10);
5+
6+
select gp_inject_fault('bump_oid', 'reset', dbid) from gp_segment_configuration where role = 'p' and content = -1;
7+
8+
select max(oid)::bigint > (power(2,31) + 1)::bigint from pg_resgroup;
9+
10+
-- count(*) > 0 to run the SQL but do not display the result
11+
select count(*) > 0 from pg_resgroup_get_status(NULL);
12+
13+
drop resource group rg_large_oid;

0 commit comments

Comments
 (0)