Skip to content

Commit 5660c4a

Browse files
[release/5.0] Fix pal cgroup v2 implementation (#45001)
* Fix pal cgroup v2 implementation Fixes two issues in src/pal/src/misc/cgroup.cpp: * No subsystem match must be performed for cgroup v2. * Incorrect arguments for sscanf_s when reading cgroup path. The src/gc/unix/cgroup.cpp implementation doesn't have these issues. * Rename is_subsystem_match to isSubsystemMatch Co-authored-by: Tom Deseyn <tom.deseyn@gmail.com>
1 parent 65a4f71 commit 5660c4a

File tree

2 files changed

+44
-36
lines changed

2 files changed

+44
-36
lines changed

src/coreclr/src/gc/unix/cgroup.cpp

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,8 @@ class CGroup
6666
static void Initialize()
6767
{
6868
s_cgroup_version = FindCGroupVersion();
69-
s_memory_cgroup_path = FindCGroupPath(&IsCGroup1MemorySubsystem);
70-
s_cpu_cgroup_path = FindCGroupPath(&IsCGroup1CpuSubsystem);
69+
s_memory_cgroup_path = FindCGroupPath(s_cgroup_version == 1 ? &IsCGroup1MemorySubsystem : nullptr);
70+
s_cpu_cgroup_path = FindCGroupPath(s_cgroup_version == 1 ? &IsCGroup1CpuSubsystem : nullptr);
7171
}
7272

7373
static void Cleanup()
@@ -257,12 +257,19 @@ class CGroup
257257

258258
if (strncmp(filesystemType, "cgroup", 6) == 0)
259259
{
260-
char* context = nullptr;
261-
char* strTok = strtok_r(options, ",", &context);
262-
while (strTok != nullptr)
260+
bool isSubsystemMatch = is_subsystem == nullptr;
261+
if (!isSubsystemMatch)
263262
{
264-
if ((s_cgroup_version == 2) || ((s_cgroup_version == 1) && is_subsystem(strTok)))
263+
char* context = nullptr;
264+
char* strTok = strtok_r(options, ",", &context);
265+
while (!isSubsystemMatch && strTok != nullptr)
265266
{
267+
isSubsystemMatch = is_subsystem(strTok);
268+
strTok = strtok_r(nullptr, ",", &context);
269+
}
270+
}
271+
if (isSubsystemMatch)
272+
{
266273
mountpath = (char*)malloc(lineLen+1);
267274
if (mountpath == nullptr)
268275
goto done;
@@ -281,9 +288,6 @@ class CGroup
281288
*pmountpath = mountpath;
282289
*pmountroot = mountroot;
283290
mountpath = mountroot = nullptr;
284-
goto done;
285-
}
286-
strTok = strtok_r(nullptr, ",", &context);
287291
}
288292
}
289293
}

src/coreclr/src/pal/src/misc/cgroup.cpp

Lines changed: 31 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,8 @@ class CGroup
5353
static void Initialize()
5454
{
5555
s_cgroup_version = FindCGroupVersion();
56-
s_memory_cgroup_path = FindCGroupPath(&IsCGroup1MemorySubsystem);
57-
s_cpu_cgroup_path = FindCGroupPath(&IsCGroup1CpuSubsystem);
56+
s_memory_cgroup_path = FindCGroupPath(s_cgroup_version == 1 ? &IsCGroup1MemorySubsystem : nullptr);
57+
s_cpu_cgroup_path = FindCGroupPath(s_cgroup_version == 1 ? &IsCGroup1CpuSubsystem : nullptr);
5858
}
5959

6060
static void Cleanup()
@@ -245,33 +245,37 @@ class CGroup
245245

246246
if (strncmp(filesystemType, "cgroup", 6) == 0)
247247
{
248-
char* context = nullptr;
249-
char* strTok = strtok_s(options, ",", &context);
250-
while (strTok != nullptr)
248+
bool isSubsystemMatch = is_subsystem == nullptr;
249+
if (!isSubsystemMatch)
251250
{
252-
if (is_subsystem(strTok))
251+
char* context = nullptr;
252+
char* strTok = strtok_s(options, ",", &context);
253+
while (!isSubsystemMatch && strTok != nullptr)
253254
{
254-
mountpath = (char*)PAL_malloc(lineLen+1);
255-
if (mountpath == nullptr)
256-
goto done;
257-
mountroot = (char*)PAL_malloc(lineLen+1);
258-
if (mountroot == nullptr)
259-
goto done;
260-
261-
sscanfRet = sscanf_s(line,
262-
"%*s %*s %*s %s %s ",
263-
mountroot, lineLen+1,
264-
mountpath, lineLen+1);
265-
if (sscanfRet != 2)
266-
_ASSERTE(!"Failed to parse mount info file contents with sscanf_s.");
267-
268-
// assign the output arguments and clear the locals so we don't free them.
269-
*pmountpath = mountpath;
270-
*pmountroot = mountroot;
271-
mountpath = mountroot = nullptr;
272-
goto done;
255+
isSubsystemMatch = is_subsystem(strTok);
256+
strTok = strtok_s(nullptr, ",", &context);
273257
}
274-
strTok = strtok_s(nullptr, ",", &context);
258+
}
259+
if (isSubsystemMatch)
260+
{
261+
mountpath = (char*)PAL_malloc(lineLen+1);
262+
if (mountpath == nullptr)
263+
goto done;
264+
mountroot = (char*)PAL_malloc(lineLen+1);
265+
if (mountroot == nullptr)
266+
goto done;
267+
268+
sscanfRet = sscanf_s(line,
269+
"%*s %*s %*s %s %s ",
270+
mountroot, lineLen+1,
271+
mountpath, lineLen+1);
272+
if (sscanfRet != 2)
273+
_ASSERTE(!"Failed to parse mount info file contents with sscanf_s.");
274+
275+
// assign the output arguments and clear the locals so we don't free them.
276+
*pmountpath = mountpath;
277+
*pmountroot = mountroot;
278+
mountpath = mountroot = nullptr;
275279
}
276280
}
277281
}
@@ -343,7 +347,7 @@ class CGroup
343347
// See https://www.kernel.org/doc/Documentation/cgroup-v2.txt
344348
// Look for a "0::/some/path"
345349
int sscanfRet = sscanf_s(line,
346-
"0::%s", lineLen+1,
350+
"0::%s",
347351
cgroup_path, lineLen+1);
348352
if (sscanfRet == 1)
349353
{

0 commit comments

Comments
 (0)