Kamalesh Babulal [Thu, 12 Mar 2026 03:39:43 +0000 (09:09 +0530)]
config: pass NULL when probing systemd default cgroup
ASan reported following global-buffer-overflow:
READ of size 1 at 0x7f50dc6b3e9f thread T0
#0 0x... in cg_concat_path <src>/src/api.c:1769
#1 0x... in cg_build_path_locked <src>/src/api.c:1889
#2 0x... in cg_build_path <src>/src/api.c:1910
#3 0x.. in systemd_default_cgroup_exists <src>/src/config.c:2258
#4 0x.. in cgroup_set_default_systemd_cgroup <src>/src/config.c:2306
#5 0x... in main <src>/src/tools/cgdelete.c:193
#6 0x... in __libc_start_call_main (/lib64/libc.so.6+0x2a60f)
#7 0x... in __libc_start_main_alias_2 (/lib64/libc.so.6+0x2a6bf)
#8 0x... in _start (/usr/local/bin/cgdelete+0x402384)
0x... sits one byte to the left of the empty string literal '.LC3' that
systemd_default_cgroup_exists() handed to cg_build_path(). Passing an
empty suffix made cg_concat_path() evaluate suf[-1] while it decided
whether to append a trailing slash, triggering the ASan
global-buffer-overflow.
Fix by passing NULL, instead of "" (empty string) so cg_build_path()
skips the suffix concatenation entirely. The resulting canonical paths
are unchanged, but cgdelete (and any other caller) now runs without
trampling the adjacent literal.
Fixes: https://github.com/libcgroup/libcgroup/issues/526 Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Kamalesh Babulal [Fri, 13 Mar 2026 16:32:45 +0000 (22:02 +0530)]
tools/cgxget: free temporary conversions on failure paths
AddressSanitizer reported an 8-byte leak when convert_cgroups() aborted:
Direct leak of 8 byte(s) in 1 object(s) allocated from:
#0 0x... in __interceptor_malloc
#1 convert_cgroups (<libcgroup-source>/src/tools/cgxget.c:822)
#2 main (<libcgroup-source>/src/tools/cgxget.c:893)
The helper allocates a scratch cgroup list, but on failures or
ECGNOVERSIONCONVERT it returned without releasing the partially built
array. Worse, when the scratch malloc failed we still treated the
conversion as “success” and left the old list pointing nowhere.
Return an error if the allocation fails, zero-initialise the scratch
list so we can free partially populated slots safely, free every
temporary cgroup before returning on error, and only after a successful
conversion free the original list (including the pointer array) before
swapping in the new one. With these guards in place the ASan leak report
is gone.
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Even after fixing per-controller leaks, ASan still reported an 8 byte
leak on exit:
Direct leak of 8 byte(s) in 1 object(s) allocated from:
#0 0x... in __interceptor_realloc
#1 create_cg (<libcgroup-source>/src/tools/cgxget.c:95)
main() freed each struct cgroup * in cgrp_list but never released the
pointer array itself, so the final realloc() stayed live. After freeing
the entries, free cgrp_list too. This fixes the sanitizer warning.
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
tools/cgxget: create cgroup before growing pointer list
AddressSanitizer showed that forcing a parse failure after "-g" leaked
the controller pointer array. create_cg() called realloc() directly on
*cgrp_list, when realloc() returned NULL we overwrote the handle to the
existing array and lost every previously allocated entry.
Rework the helper so it builds the new struct cgroup first and only
assigns to *cgrp_list after realloc() succeeds. If the resize fails we
free the freshly allocated cgroup and leave the original pointer array
intact. On success we append the new pointer and bump the length as
before, keeping the parser's rollback path leak-free.
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
tools/cgxget: free controller tokens and revert cgroup on -g failure
AddressSanitizer reported two 8-byte leaks when handling cgxget "-g"
controller:<cgroup>:
Direct leak of 8 byte(s) in 1 object(s) allocated from:
#0 0x... in __interceptor_realloc
#1 split_controllers (<libcgroup-source>/src/tools/cgxget.c:269)
Direct leak of 8 byte(s) in 1 object(s) allocated from:
#0 0x... in __interceptor_realloc
#1 create_cg (<libcgroup-source>/src/tools/cgxget.c:88)
split_controllers() grew the controller array with realloc()/strdup()
but on failure it leaked every already-duplicated token, and the caller
never freed the temporary array even on success.
parse_g_flag_with_colon() also pushed a new struct cgroup onto the list
and left it behind whenever a later step bailed out, so the cgroup
object stayed allocated. The helper now frees the duplicated controller
tokens on error, releases the scratch array after use, and if parsing
fails it frees the freshly appended cgroup and drops the length so the
caller ignores it. With these changes the ASan leak report disappears.
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Even after fixing per-controller leaks, ASan still reported an 8 byte
leak on exit:
Direct leak of 8 byte(s) in 1 object(s) allocated from:
#0 0x... in __interceptor_realloc
#1 create_cgrp (<libcgroup-source>/src/tools/cgget.c:76)
main() freed each struct cgroup * in cgrp_list but never released the
pointer array itself, so the final realloc() stayed live. After freeing
the entries, free cgrp_list too. This fix the sanitizer warning.
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
tools/cgget: create cgroup before growing pointer list
AddressSanitizer showed that forcing a parse failure after "-g" leaked
the controller pointer array. create_cgrp() called realloc() directly on
*cgrp_list, when realloc() returned NULL we overwrote the handle to the
existing array and lost every previously allocated entry.
Rework the helper so it builds the new struct cgroup first and only
assigns to *cgrp_list after realloc() succeeds. If the resize fails we
free the freshly allocated cgroup and leave the original pointer array
intact. On success we append the new pointer and bump the length as
before, keeping the parser's rollback path leak-free.
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Kamalesh Babulal [Sat, 21 Feb 2026 02:48:30 +0000 (08:18 +0530)]
tools/cgget: free controller tokens and revert cgroup on -g failure
AddressSanitizer reported two 8-byte leaks when handling cgget -g
controller:<cgroup>:
Direct leak of 8 byte(s) in 1 object(s) allocated from:
#0 0x... in __interceptor_realloc
#1 split_controllers (<libcgroup-source>/src/tools/cgget.c:257)
Direct leak of 8 byte(s) in 1 object(s) allocated from:
#0 0x... in __interceptor_realloc
#1 create_cgrp (<libcgroup-source>/src/tools/cgget.c:76)
split_controllers() grew the controller array with realloc()/strdup()
but on failure it leaked every already-duplicated token, and the caller
never freed the temporary array even on success.
parse_g_flag_with_colon() also pushed a new struct cgroup onto the list
and left it behind whenever a later step bailed out, so the cgroup object
stayed allocated. The helper now frees the duplicated controller tokens
on error, releases the scratch array after use, and if parsing fails it
frees the freshly appended cgroup and drops the length so the caller
ignores it. With these changes the ASan leak report disappears.
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Kamalesh Babulal [Sat, 14 Mar 2026 05:04:23 +0000 (10:34 +0530)]
api: leave room for NUL in cgroup_get_procs()
Commit 4a851a569689 ("api: Fix unsafe call to strncat in
cgroup_get_procs() and cgroup_get_threads()") tightened several
strncat() callers, but the code building the procs/threads paths
still used the raw remaining buffer size. When the buffer is
already full, strncat() would copy path_sz bytes and overwrite
the terminator. Subtract one from the remaining length before
appending "tasks" or the controller-specific filename so there
is always space for the trailing NUL.
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Kamalesh Babulal [Sun, 15 Mar 2026 05:32:56 +0000 (11:02 +0530)]
ftests/999: tidy config writes and stress paths
Systemd.write_config_with_pid() now writes the temporary config inside a
context manager so the file descriptor is always closed. The 999-stress
helper switched to os.path.join(), which automatically uses the right
separator, makes the intent clearer than manual '/' concatenation, and
guards future edits from producing malformed paths.
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
ftests/077: drop unused global and fix list sorting
flake8 reported:
tests/ftests/077-pybindings-cgroup_get_procs.py:52:5: F824 is unused:
name is never assigned in scope
The test already appends to the module-level list, so we do not reassign
it inside setup(). Drop the unused global declaration and call
list.sort() without reassigning the result to keep the pid lists in order
before comparing them.
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
flake8 reported:
tests/ftests/process.py:276:12: E721 do not compare types, for exact
checks use is / is not, for instance checks use isinstance()
tests/ftests/process.py:278:14: E721 do not compare types, for exact
checks use is / is not, for instance checks use isinstance()
Process.kill() only needs to normalise string or integer inputs into a
list before signalling, so switch the raw type() comparisons to
isinstance() calls. Behaviour stays the same and the lint warning
disappears.
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
flake8 reported:
tests/ftests/cgroup.py:1171:33: E721 do not compare types, for exact
checks use is / is not, for instance checks use isinstance()
tests/ftests/cgroup.py:1173:14: E721 do not compare types, for exact
checks use is / is not, for instance checks use isinstance()
The helper only needs to distinguish between string, list, or None
inputs, so replace the raw type() comparisons with isinstance() calls.
That keeps the behaviour unchanged while removing the lint warning.
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Kamalesh Babulal [Sat, 17 Jan 2026 05:09:17 +0000 (10:39 +0530)]
tests/ftests: speed up is_output_same() line compare loop
is_output_same() used to call expected_out.splitlines() on every
iteration and index into the freshly built list. Switching to
enumerate(zip(out_lines, exp_lines)) lets us split both strings once
and walk the pairs directly.
A quick timeit run (1,000,000 iterations with matching output) shows
the new loop shaving ~5% off the runtime:
Kamalesh Babulal [Fri, 16 Jan 2026 11:39:35 +0000 (17:09 +0530)]
src/config: free namespace table on success and failure
The per-thread namespace table (cg_namespace_table[]) is populated by
config_order_namespace_table() / config_validate_namespaces() under
cg_mount_table_lock and then consumed by cg_build_path_locked().
Before this patch we reset the array with a bespoke loop + memset at the
front of config_order_namespace_table(), but error paths relied on the
call chain eventually unwinding into cgroup_free_config() to release
any partially populated entries. That worked, yet it meant the table
could briefly hold dangling pointers after an error while the lock was
already dropped, one subtle slip or new return path would have left stale
namespaces visible to the rest of the library.
This change introduces a small helper,
cgroup_config_free_namespaces_table(), that walks the thread-local
array, frees each string, and nulls the slot. We invoke it in three
places:
- right at the start of config_order_namespace_table(), so every run
begins with a clean slate.
- from the error branches of both namespace helpers, guaranteeing that
even a mid-stream failure leaves the table empty before the lock is
released.
- and (unchanged) from cgroup_free_config(), so the happy path tears
everything down once the config work finishes.
The end result is both cleaner and safer: we delete the redundant
set-to-NULL/memset sequence and the namespace table is always in a known
state no matter how the parser exits.
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Add explicit '\0' writes after the strncpy() calls that populate the
local group and target buffers so subsequent new_cgroup() and
strncat() calls always see bounded, terminated strings.
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Kamalesh Babulal [Fri, 30 Jan 2026 06:41:54 +0000 (12:11 +0530)]
tools/cgxget: terminate cgroup names after strncpy()
split_cgroup_name() and parse_opt_args() copy user-supplied names
into struct cgroup->name with strncpy(), but never add a trailing
'\0'. The structs are zeroed when created via cgroup_new_cgroup(""),
yet if we ever reuse an existing object the truncated name would remain
unterminated.
Fix this by explicitly setting cgrp_name[FILENAME_MAX - 1] = '\0' in
split_cgroup_name() and writing '\0' to the last byte of each
destination buffer in parse_opt_args() so the names are always
NUL-terminated.
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Kamalesh Babulal [Thu, 29 Jan 2026 06:41:29 +0000 (12:11 +0530)]
tools/cgget: terminate cgroup names after strncpy()
split_cgroup_name() and the parse_opt_args() paths copy user-provided
cgroup names with strncpy() but never force a trailing '\0'. Today those
structs come from cgroup_new_cgroup(""), so they start zeroed, but the
code relies on that implementation detail. If we ever reuse an existing
struct cgroup or the initializer changes, truncated names would remain
unterminated.
Guard against that by explicitly writing '\0' to the last byte after
each copy, ensuring cgrp->name stays well-formed regardless of how the
struct was allocated.
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Kamalesh Babulal [Wed, 28 Jan 2026 06:15:36 +0000 (11:45 +0530)]
src/api: terminate controller copies in cgroup_copy_controller_values()
cgroup_copy_controller_values() populates the destination controller
structs with strncpy(), but never forces a trailing '\0'. Today the
destinations are zeroed via calloc(), but reusing an existing struct or
copying a token that fills the buffer would leave dst->name and each
control_value dst_val.{name,value} unterminated.
Guard against that by explicitly writing '\0' to the last byte of each
destination buffer after the copy.
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
config_insert_cgroup() copies the parsed group name into
config_cgrp->name with strncpy(), but never writes a trailing '\0'.
If the name is exactly FILENAME_MAX - 1 bytes long and this entry is
reused while reloading the configuration, the buffer stays unterminated
and callers can read garbage past the end.
Fix it by explicitly setting config_cgrp->name[FILENAME_MAX - 1] = '\0'
after the copy.
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Kamalesh Babulal [Mon, 26 Jan 2026 05:54:32 +0000 (11:24 +0530)]
src/api: terminate strings in cg_read_stat()
In cg_read_stat() strncpy() is used to split the stats into token and
copying them into struct cgroup_stat.{name,value}, but are not terminated
with a trailing '\0'. If the stat token length matched the buffer, the
stack-allocated struct will stay unterminated, risking stray data on
later use.
Fix it by explicitly terminating both cgroup_stat.{name, value} with
'\0'.
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
AddressSanitizer reported that the v2 test leaked the struct cgroup
returned by cgroup_new_cgroup():
Indirect leak of 4928 byte(s) in 1 object(s) allocated from:
#0 0x... in calloc
#1 cgroup_new_cgroup (<source>/libcgroup/src/wrapper.c:50)
#2 CgroupCreateCgroupTest_CgroupCreateCgroupV2_Test::TestBody()
The test was allocating a cgroup, populating controllers, and then
exiting without calling the cgroup_free() helper, so all nested
controller state stayed allocated. Fix it by calling cgroup_free(&cgrp)
in each create_cgroup before returning, which releases the controller
list and silences the ASan warning.
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Kamalesh Babulal [Sat, 28 Feb 2026 11:09:17 +0000 (16:39 +0530)]
gunit/003: free controller lists returned by cg_get_cgroups_from_proc_cgroups()
LeakSanitizer spotted leak in the ReadExampleFile test. Where every
call to cg_get_cgroups_from_proc_cgroups() mallocs controller/cgroup
strings, but never released them:
Direct leak of 147 byte(s) in 7 object(s) allocated from:
#0 0x... in __interceptor_malloc
#1 cg_get_cgroups_from_proc_cgroups
(/<home>/libcgroup/src/api.c:5934)
Fix it by adding a FreeLists() helper and invoke it at the end of each
test case, so every populated slot in controller_list[] and cgrp_list[]
gets free()'d and nulled. Header-wise, <stdlib.h> is included for free().
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Kamalesh Babulal [Fri, 27 Feb 2026 08:43:12 +0000 (14:13 +0530)]
gunit/006: fix ASan leak by calling cgroup_free()
CgroupGetCgroup1 tripped LeakSanitizer because the test freed the
top-level struct cgroup with free(), leaving the controllers and control
values allocated by cgroup_get_cgroup() on the heap:
Direct leak of 4240 byte(s) in 5 object(s) allocated from:
#0 0x... in calloc
#1 cgroup_add_controller (<source>/libcgroup/src/wrapper.c:79)
#2 cgroup_get_cgroup (<source>/libcgroup/src/api.c:3973)
Replacing both free(cgrp) calls with cgroup_free(&cgrp) mirrors the
expected teardown path, unwinds the nested allocations, and clears
the ASan report without changing the test assertions.
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Kamalesh Babulal [Thu, 26 Feb 2026 06:39:37 +0000 (12:09 +0530)]
gunit/017: release fuzz cgroups during test cleanup
ASan reported that APIArgsTest_API_cgroup_get_uid_gid_Test leaked the
struct cgroup returned by cgroup_new_cgroup():
==2840589==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 4928 byte(s) in 1 object(s) allocated from:
.../tests/gunit/017-API_fuzz_test.cpp:287
APIArgsTest constructs several valid cgroups but doesn’t release them,
so each sanitizer run leaves their heap allocations behind. The test
now tracks the cgroups it creates via a NewTestCgroup() helper and
releases them from TearDown(), and all positive cgroup_new_cgroup()
call sites in the file use that helper. This mirrors the expected
cgroup_free() cleanup, keeps the fuzz-style tests self-contained, and
silences the ASan leak report.
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Kamalesh Babulal [Wed, 25 Feb 2026 04:50:27 +0000 (10:20 +0530)]
gunit/008: reset v2 mount and free duplicate mounts
ASan caught the v2 mount path, leaking the cg_mount_point nodes
created by cg_add_duplicate_mount() and the strdup()ed mount paths the
tests stash in struct mntent. Running make -C tests/gunit check reported:
==2840589==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 28728 byte(s) in 7 object(s) allocated from:
.../tests/gunit/008-cgroup_process_v2_mount.cpp:133
This patch adds a private ResetMountTable() helper that takes the
mount table write lock, walks each controllers linked list to free the
heap nodes, zeroes cg_mount_table, clears cg_cgroup_v2_mount_path, and
resets mnt_tbl_idx. SetUp() and TearDown() call it so each test starts
from a clean slate. The duplicate test now runs the initial mounting
step itself before it exercises the duplicate code path, and every test
frees the temporary mnt_dir strings after the checks are done.
The cleanup path now, mirrors cgroup_free_cg_mount_table() and silences
the ASan leak report while keeping the tests self-contained.
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Kamalesh Babulal [Tue, 24 Feb 2026 03:34:58 +0000 (09:04 +0530)]
gunit/009: free ctrlr values in recursive set-values test
ASan flagged a leak in SetValuesRecursiveTest_SuccessfulSetValues
because the test allocates controller values with calloc() and never
releases them:
==2840589==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 32864 byte(s) in 4 object(s) allocated from:
#0 0x... in calloc
#1 SetValuesRecursiveTest_SuccessfulSetValues_Test::TestBody()
.../tests/gunit/009-cgroup_set_values_recursive.cpp:92
This patch fixes it by looping over ctrlr.values[], free()ing each
entry, setting the pointer to NULL, and resetting ctrlr.index so the
stack allocated controller struct is clean when the test exits.
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Kamalesh Babulal [Mon, 23 Feb 2026 06:06:08 +0000 (11:36 +0530)]
systemd,cgcreate: use strchr() to find the slice/scope separator
Both cgroup_create_scope2() and create_systemd_scope() used strstr() to
locate the '/' delimiter between slice and scope components. Since we
are matching a single character, use strchr() instead to express the
intent precisely and avoid the extra substring search semantics/overhead
implied by strstr().
No functional change expected. cgroup name that do not conform to
<slice-name>.slice/<scope-name>.scope, continue to be rejected.
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Kamalesh Babulal [Sat, 10 Jan 2026 06:42:09 +0000 (12:12 +0530)]
ftests: Remove unnecessary use of global in Log::log()
The global statement is only required when assigning to a global
variable, not when simply reading its value. In Log::log(), the
global log_level, global log_file and global log_fd declarations
are unnecessary because these variables are only read. Remove both
declarations from Log::log().
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
ftests: Remove unnecessary use of global in run_tests()
The global statement is only required when assigning to a global
variable, not when simply reading its value. In run_tests(), the global
setup_time and global teardown_time declarations are unnecessary
because these variables are only read. Remove both declarations from
run_tests().
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
ftests/077: Remove unnecessary use of global in teardown()
The global statement is only needed when assigning to a global variable,
not when only reading its value. In teardown(), the global
initial_pid_list declaration is unnecessary since pid is only being
read. Remove the global initial_pid_list from teardown().
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
ftests/068: Remove unnecessary use of global in teardown()
The global statement is only required when assigning to a global
variable, not when simply reading its value. In teardown(), the global
SYSTEMD_PIDS and global OTHER_PIDS declarations are unnecessary because
these variables are only read. Remove both declarations from teardown().
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
ftests/067: Remove unnecessary use of global in teardown()
The global statement is only required when assigning to a global
variable, not when simply reading its value. In teardown(), the global
SYSTEMD_PIDS and global OTHER_PIDS declarations are unnecessary because
these variables are only read. Remove both declarations from teardown().
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
ftests/066: Remove unnecessary use of global in teardown()
The global statement is only required when assigning to a global
variable, not when simply reading its value. In teardown(), the global
SYSTEMD_PIDS and global OTHER_PIDS declarations are unnecessary because
these variables are only read. Remove both declarations from teardown().
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
ftests/065: Remove unnecessary use of global in teardown()
The global statement is only required when assigning to a global
variable, not when simply reading its value. In teardown(), the global
SYSTEMD_PIDS and global OTHER_PIDS declarations are unnecessary because
these variables are only read. Remove both declarations from teardown().
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
ftests/058: Remove unnecessary use of global in teardown()
The global statement is only needed when assigning to a global variable,
not when only reading its value. In teardown(), the global pid
declaration is unnecessary since pid is only being read. Remove the
global pid from teardown().
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
ftests/050: Remove unnecessary use of global in teardown()
The global statement is only needed when assigning to a global variable,
not when only reading its value. In teardown(), the global pid
declaration is unnecessary since pid is only being read. Remove the
global pid from teardown().
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Tom Hromatka [Mon, 12 Jan 2026 16:41:47 +0000 (09:41 -0700)]
api: Fix unsafe call to strncat in cgroup_get_procs() and cgroup_get_threads()
TJH - the text below was autogenerated by Copilot.
In general, when using strncat, the third argument must reflect the
remaining space in the destination buffer minus one byte to keep room
for the terminating NUL. The correct upper bound is therefore
sizeof(dest) - strlen(dest) - 1. This ensures strncat cannot write past
the end of the buffer, even including the terminator it always appends.
For this code, the minimal, behavior-preserving fix is to adjust the
strncat calls that append constant suffixes to cgroup_path.
Specifically:
In cgroup_get_procs, change FILENAME_MAX - strlen(cgroup_path) to
FILENAME_MAX - strlen(cgroup_path) - 1.
In cgroup_get_threads, make the same adjustment.
No other logic needs to change; the functions will still append the same
suffixes, but the maximum number of characters strncat is allowed to
copy will correctly reserve one byte for the NUL terminator. If
cg_build_path already fills nearly the entire buffer, the new limit
prevents overflow and may result in a truncated path; if such truncation
should be handled explicitly, additional error checks on
strlen(cgroup_path) relative to FILENAME_MAX could be added, but that
would go beyond the minimal fix requested.
These changes are all within src/api.c, in the region containing
cgroup_get_procs and cgroup_get_threads, and do not require any new
includes or helper functions.
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com> Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com>
src/api.c: potential null ptr deref in cg_get_cgroups_from_proc_cgroups
In function cg_get_cgroups_from_proc_cgroups there is an allocation of memory by calling
malloc(buff_len).
Result of this allocation is not checked,
and in case of OOM it'll lead to null ptr deref.
This commit adds handling of possible null ptr as a result of failed
memory allocation.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
src/api.c: prevent potential null ptr deref in cgroup_fill_cgc
In function cgroup_fill_cgc there is an allocation of memory by calling
strdup(ctrl_dir->d_name). Result of this allocation is not checked,
and in case of OOM it'll lead to null ptr deref.
This commit adds handling of possible null ptr as a result of failed
memory allocation.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Александр Ушаков [Mon, 28 Jul 2025 10:23:12 +0000 (13:23 +0300)]
src/lex.l: fix pointer overflow in yylex()
UBSAN reported a pointer overflow bug when a fuzz test passed empty
strings to cgroup_init_templates_cache(). The issue is triggered by
the strlen(yylval.name - 1) check, which returns a negative value.
This value is then implicitly cast to an unsigned long long, causing
incorrect behavior. Fix this by adding checks for empty strings inputs.
This issue was discovered while running fuzz tests using the Clang
compiler.
[Kamalesh added commit message] Signed-off-by: Aleksandr Ushakov <aushakov@astralinux.ru> Acked-by: Tom Hromatka <tom.hromatka@oracle.com> Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com>
src: make CGRP_RULE_MAXLINE include CG_OPTIONS_MAX
CGRP_RULE_MAXLINE const, which is used as a length for buffer for
unparsed cgroup rules strings, doesn't include length of options
field (CG_OPTIONS_MAX). Therefore buffer length is not enough
to store all rule fields.
Commit is aimed to include CG_OPTIONS_MAX in CGRP_RULE_MAXLINE
to avoid potential problems with rules parsing.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
[Kamalesh introduced tabs to align with macro continuation] Signed-off-by: Mikhail Dmitrichenko <m.dmitrichenko222@gmail.com> Acked-by: Tom Hromatka <tom.hromatka@oracle.com> Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com>
Remove the duplicate `if` check. No functional change.
Fixes: 1545d4df54ea7 ("ftests/009: Refactor code to match outputs with same line") Acked-by: Tom Hromatka <tom.hromatka@oracle.com> Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com>
Kamalesh Babulal [Mon, 26 May 2025 05:29:47 +0000 (10:59 +0530)]
tests/gunit: disable execution with --gtest_shuffle
Some Google tests rely on being run in a specific order, with certain
tests depending on the successful completion of previous ones. Passing
the --gtest_shuffle option can break this sequence, potentially leading
to test failures or inconsistent results.
Until the test suite is updated to support randomized execution, skip
running tests when --gtest_shuffle is used.
Acked-by: Tom Hromatka <tom.hromatka@oracle.com> Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com>
Tom Hromatka [Tue, 29 Jul 2025 16:41:15 +0000 (16:41 +0000)]
cgclassify: Fix clang warnings
Fix the following clang warnings by shrinking the scan size from 4096 to
4095 to allow room for the null terminating character.
cgclassify.c:334:47: warning: 'sscanf' may overflow; destination buffer
in argument 4 has size 4096, but the corresponding specifier may require
size 4097 [-Wfortify-source]
334 | ret = sscanf(buffer, "%d::%4096s\n",
&idx, cgrp_name);
|
^
cgclassify.c:336:63: warning: 'sscanf' may overflow; destination buffer
in argument 5 has size 4096, but the corresponding specifier may require
size 4097 [-Wfortify-source]
336 | ret = sscanf(buffer,
"%d:%[^:]:%4096s\n", &idx, ctrl_name, cgrp_name);
|
^
cgclassify.c:497:40: warning: 'sscanf' may overflow; destination buffer
in argument 3 has size 4096, but the corresponding specifier may require
size 4097 [-Wfortify-source]
497 | ret = sscanf(buffer, "%*s %4096s\n", cgrp_path);
| ^
Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com> Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com>
Tom Hromatka [Tue, 29 Jul 2025 16:18:53 +0000 (16:18 +0000)]
cgexec: Fix clang warnings
Fix the following clang warnings by shrinking the scan size from 4096 to
4095 to allow room for the null terminating character.
cgexec.c:294:47: warning: 'sscanf' may overflow; destination buffer in
argument 4 has size 4096, but the corresponding specifier may require
size 4097 [-Wfortify-source]
294 | ret = sscanf(buffer, "%d::%4096s\n",
&idx, cgrp_name);
|
^
cgexec.c:296:63: warning: 'sscanf' may overflow; destination buffer in
argument 5 has size 4096, but the corresponding specifier may require
size 4097 [-Wfortify-source]
296 | ret = sscanf(buffer,
"%d:%[^:]:%4096s\n", &idx, ctrl_name, cgrp_name);
|
^
cgexec.c:423:40: warning: 'sscanf' may overflow; destination buffer in
argument 3 has size 4096, but the corresponding specifier may require
size 4097 [-Wfortify-source]
423 | ret = sscanf(buffer, "%*s %4096s\n", cgrp_path);
| ^
Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com> Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com>
Tom Hromatka [Tue, 29 Jul 2025 16:13:46 +0000 (16:13 +0000)]
api: Fix clang warning
Fix the following clang warning by using NULL instead of '\0'
api.c:6721:17: warning: expression which evaluates to zero treated as a
null pointer constant of type 'char *' [-Wnon-literal-null-conversion]
6721 | mnt_paths[i] = '\0';
| ^~~~
Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com> Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com>
Tom Hromatka [Tue, 29 Jul 2025 16:12:21 +0000 (16:12 +0000)]
api: Fix clang warnings
Fix the following clang warnings by reducing the scan size from 4096 to
4095 so that there is room for the null character.
api.c:5173:52: warning: 'fscanf' may overflow; destination buffer in
argument 4 has size 4096, but the corresponding specifier may require
size 4097 [-Wfortify-source]
5173 | ret = fscanf(pid_cgrp_fd,
"%d::%4096s\n", &num, cgrp_path);
|
^
api.c:5218:69: warning: 'fscanf' may overflow; destination buffer in
argument 5 has size 4096, but the corresponding specifier may require
size 4097 [-Wfortify-source]
5218 | ret = fscanf(pid_cgrp_fd, "%d:%[^:]:%4096s\n",
&num, controllers, cgrp_path);
|
^
Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com> Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com>
Tom Hromatka [Tue, 29 Jul 2025 16:09:05 +0000 (16:09 +0000)]
api: Fix clang warning
Fix the following clang warnings by initializing the char pointers to
NULL rather than '\0';
api.c:4232:46: warning: expression which evaluates to zero treated as a
null pointer constant of type 'char *' [-Wnon-literal-null-conversion]
4232 | char *controller_list[MAX_MNT_ELEMENTS] = { '\0' };
| ^~~~
api.c:4233:40: warning: expression which evaluates to zero treated as a
null pointer constant of type 'char *' [-Wnon-literal-null-conversion]
4233 | char *cgrp_list[MAX_MNT_ELEMENTS] = { '\0' };
| ^~~~
Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com> Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com>
Tom Hromatka [Tue, 29 Jul 2025 16:07:27 +0000 (16:07 +0000)]
api: Fix clang warning
Fix the following warnings from clang by reducing the size passed to
strncat by 1 as recommended by clang.
api.c:3784:24: warning: the value of the size argument in 'strncat' is
too large, might lead to a buffer overflow [-Wstrncat-size]
3784 | strncat(path, d_name, sizeof(path) - strlen(path));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
api.c:3784:24: note: change the argument to be the free space in the
destination buffer minus the terminating null byte
3784 | strncat(path, d_name, sizeof(path) - strlen(path));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
| sizeof(path) - strlen(path) - 1
Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com> Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com>
Tom Hromatka [Tue, 29 Jul 2025 16:04:41 +0000 (16:04 +0000)]
api: Fix clang warning
Fix the following warnings from clang by reducing the size passed to
strncat by 1 as recommended by clang.
api.c:3727:22: warning: the value of the size argument in 'strncat' is
too large, might lead to a buffer overflow [-Wstrncat-size]
3727 | strncat(path, file, sizeof(path) - strlen(path));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
api.c:3727:22: note: change the argument to be the free space in the
destination buffer minus the terminating null byte
3727 | strncat(path, file, sizeof(path) - strlen(path));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
| sizeof(path) - strlen(path) - 1
Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com> Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com>
Tom Hromatka [Tue, 29 Jul 2025 16:01:54 +0000 (16:01 +0000)]
api: Fix clang warning
Fix the following warning by simply deleting the line. The variable
path_dir_end is not used after this line, and thus does not need to be
set.
api.c:2384:19: warning: expression which evaluates to zero treated as a
null pointer constant of type 'char *' [-Wnon-literal-null-conversion]
2384 | path_dir_end = '\0';
Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com> Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com>
cgrulesengd -n (nodaemon mode) runs indefinitely until it is terminated
with SIGKILL. To handle this, introduce a 20-second timeout that is
passed to __run(), which in turn calls subproc.kill() to terminate
cgrulesengd once the timer expires.
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Fix the false positive in the testcase number 013, the return value
should be by default TEST_FAILED, instead of TEST_PASSED. The flow of
the testcase is designed to set the return value to TEST_PASSED only
if the current kernel's cpu controller settings matches any of the
expected cpu controller output list.
Fixes: 6bea4df6d283 ("ftests/013: Refactor code to match outputs with same line") Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Fix the false positive in the testcase number 010, the return value
should be by default `TEST_FAILED`, instead of `TEST_PASSED`. The flow
of the testcase is designed to set the return value to `TEST_PASSED`
only if the current kernel's cpu controller settings matches any of
the expected cpu controller output list.
Fixes: 88ffe562376f ("ftests/010: Refactor code to match outputs with same line") Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
ftests/090: Enable cpuset for root.cgroup.subtree_controller
Some version of systemd doesn't enable 'cpuset' controller for the
cgroups below default hierarchy root. Enable it, before stressing '-R'
option on cgroup v2.
-----------------------------------------------------------------
Test Results:
Run Date: Jul 09 10:24:25
Passed: 1 test(s)
Skipped: 0 test(s)
Failed: 0 test(s)
-----------------------------------------------------------------
Timing Results:
Test Time (sec)
-------------------------------------------
setup 0.00
090-cgxset-recursive_flag.py 0.27
teardown 0.00
-------------------------------------------
Total Run Time 0.27
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
ftests/013: Extend the stress to test pids.events.local (v2)
Extend the test case to exercise the pids.events.local (v2) setting
added to pid controller consts list.
-----------------------------------------------------------------
Test Results:
Run Date: Jul 09 09:56:48
Passed: 1 test(s)
Skipped: 0 test(s)
Failed: 0 test(s)
-----------------------------------------------------------------
Timing Results:
Test Time (sec)
--------------------------------------------
setup 0.00
013-cgget-multiple_g_flags.py 0.12
teardown 0.00
--------------------------------------------
Total Run Time 0.12
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Upstream Linux commit 3f26a885a068 ("cgroup/pids: Add
pids.events.local") introduced a new cgroup v2 only pids controller
setting pids.events.local. Append pid list with the newly introduced
setting.
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
api.c: prevent array out-of-bounds access in cgroup_parse_rules_file
In the function src/api.c/cgroup_parse_rules_file, the condition of loop:
for (i = 0; lst->tail->controllers[i]; i++)
cgroup_dbg(" %s", lst->tail->controllers[i]);
allows accessing lst->tail->controllers[MAX_MNT_ELEMENTS] if
lst->tail->controllers is full and lacks a terminating NULL.
Add explicit bounds checking (i < MAX_MNT_ELEMENTS) while maintaining
the NULL check. This ensures that there will never be reading past the
array boundaries regardless of its content.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
api.c: prevent array out-of-bounds access in cgroup_create_template_group
In the function src/api.c/cgroup_create_template_group,
the loop condition:
while (tmp->controllers[i] != NULL) {
allows accessing tmp->controllers[MAX_MNT_ELEMENTS] if tmp->controllers
is full and lacks a terminating NULL.
Add explicit bounds checking (i < MAX_MNT_ELEMENTS) while maintaining
the NULL check. This ensures that there will never be reading past
the array boundaries regardless of its content.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
ftests/run.py: terminate processes that exceed timeout
Some commands, like 'cgrulesengd' in non-daemon mode, continue running
until explicitly terminated. Currently, we catch exceptions for timeouts
but do not terminate the lingering process. This patch ensures that such
processes are properly killed after the timeout.
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Teodor Milkov [Thu, 12 Jun 2025 09:47:41 +0000 (15:17 +0530)]
src/config: Fix segfault while using templates
An intermittent segmentation fault occurs when classifying tasks using
cgrulesengd in combination with a template defined in cgconfig.conf.
Backtrace:
(gdb) bt
#0 __strcmp_evex () at ../sysdeps/x86_64/multiarch/strcmp-evex.S:295
#1 0x00007079d2e69645 in cgroup_config_create_template_group (cgroup=0x18844f834600,
template_name=0x18844f710cd0 "system/hosting-users/%g/mail-delivery", flags=1) at config.c:1950
#2 0x00007079d2e61458 in cgroup_create_template_group (orig_group_name=0x79905b6ebb70 "system/hosting-users/ryleeisitt/mail-delivery",
tmp=0x18844f69cce0, flags=1) at api.c:4609
#3 0x00007079d2e61bcd in cgroup_change_cgroup_flags (uid=0, gid=674, procname=0x18844f79c310 "/var/qmail/bin/qmail-queue-wrapper", pid=1048, flags=1)
at api.c:4811
#4 0x000018844cc0f29d in cgre_process_event (ev=0x79905b6ecc74, type=4) at cgrulesengd.c:485
#5 0x000018844cc0f441 in cgre_handle_msg (cn_hdr=0x79905b6ecc60) at cgrulesengd.c:531
#6 0x000018844cc0f678 in cgre_receive_netlink_msg (sk_nl=0) at cgrulesengd.c:596
#7 0x000018844cc0ffe1 in cgre_create_netlink_socket_process_msg () at cgrulesengd.c:786
#8 0x000018844cc10e4e in main (argc=3, argv=0x79905b6eda58) at cgrulesengd.c:1282
Fix the issue using cgroup->index that holds the count of controllers,
populated for the cgroup instead of checking for
cgroup->controller[index] is NULL.
ftests/utils: add get_distro() to detect Linux distribution
Introduce helper function get_distro() to identify the current Linux
distribution. Currently checks for 'Ubuntu' and 'Oracle Linux', which
are the only distros used for testing. Can be extended to support others
in the future.
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Kamalesh Babulal [Fri, 13 Jun 2025 12:38:36 +0000 (18:08 +0530)]
src: Bump MAX_MNT_ELEMENTS for new 'dmem' controller
Upstream commit b168ed458ddec ("kernel/cgroup: Add 'dmem' memory
accounting cgroup") adds a new controller. Update MAX_MNT_ELEMENTS
to match the increased number of cgroup controllers.
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Sam James [Thu, 22 May 2025 23:51:01 +0000 (00:51 +0100)]
tests: fix checking wrong result in 017-API_fuzz_test
There seem to be various calls to cgroup_add_controller where the result
isn't validated, instead checking the cgrp pointer.
We could add two asserts in each place being changed here if the concern
was testing for if cgroup_add_controller mangled cgrp, but it looks
more likely that it's a copy/paste error instead and we're just testing
the wrong thing.
Signed-off-by: Sam James <sam@gentoo.org> Acked-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Sam James [Fri, 23 May 2025 00:13:51 +0000 (01:13 +0100)]
src: handle NULL options in cgroup_parse_rules_options
We don't want to pass NULL to strtok:
```
[ RUN ] ParseRulesOptionsTest.RulesOptions_NullOptions
==2006496== Conditional jump or move depends on uninitialised value(s)
==2006496== at 0x520D156: strtok_r (strtok_r.c:49)
==2006496== by 0x403FB7E: cgroup_parse_rules_options (api.c:529)
==2006496== by 0x4005F41: ParseRulesOptionsTest_RulesOptions_NullOptions_Test::TestBody() (002-cgroup_parse_rules_options.cpp:89)
==2006496== by 0x4ABA45D: ??? (in /usr/lib64/libgtest.so.1.15.2)
==2006496== by 0x4ABE5AB: ??? (in /usr/lib64/libgtest.so.1.15.2)
==2006496== by 0x4A9D949: testing::TestInfo::Run() (in /usr/lib64/libgtest.so.1.15.2)
==2006496== by 0x4ABF18A: ??? (in /usr/lib64/libgtest.so.1.15.2)
==2006496== by 0x4AB7467: testing::internal::UnitTestImpl::RunAllTests() (in /usr/lib64/libgtest.so.1.15.2)
==2006496== by 0x4AACC46: testing::UnitTest::Run() (in /usr/lib64/libgtest.so.1.15.2)
==2006496== by 0x4002345: UnknownInlinedFun (gtest.h:2334)
==2006496== by 0x4002345: main (gtest.cpp:15)
==2006496==
Error: failed to parse options: (null)
```
Signed-off-by: Sam James <sam@gentoo.org> Acked-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Tom Hromatka [Mon, 14 Apr 2025 20:54:50 +0000 (14:54 -0600)]
ftests: Add a test for the memory abstraction layer
Add a test for the memory abstraction layer abstractions.
Currently only supports:
memory.max <-> memory.limit_in_bytes
memory.high <-> memory.soft_limit_in_bytes
-----------------------------------------------------------------
Test Results:
Run Date: Apr 14 14:52:47
Passed: 1 test(s)
Skipped: 0 test(s)
Failed: 0 test(s)
-----------------------------------------------------------------
Timing Results:
Test Time (sec)
--------------------------------------------
setup 0.00
093-cgxget-memory_settings.py 3.34
teardown 0.00
--------------------------------------------
Total Run Time 3.34
[Kamalesh removed the unused exceptions variable (lint warnings)] Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com> Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com>
Update the release process documentation to include the missing step for
pushing to the newly created release branch. Renumber subsequent steps
to reflect the correct order.
Suggested-by: Tom Hromatka <tom.hromatka@oracle.com> Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
TJH: Added "If this is a new..." text
Tom Hromatka [Tue, 8 Apr 2025 23:19:40 +0000 (17:19 -0600)]
github: dist: Rework the unit tests
Add a configure option to enable/disable the unit tests. Because of their
dependency upon googletest, they're very difficult to run for distros.
This allows them to packaged with libcgroup, but they are disabled by
default. They can be enabled via the config option `--enable-unittests`.
Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com> Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com>
Tom Hromatka [Tue, 8 Apr 2025 22:42:23 +0000 (16:42 -0600)]
github: dist: Rework the automated functional tests
Now that Github Actions don't support cgroup v1, we can make the github
runners run the tests in a more granular fashion. Add a runner to
individually run the containerized tests, the non-containerized tests,
and the sudo tests. This simplifies the testing logic, and should help
us better identify failures.
As part of this, no longer run ftests-wrapper.sh when `make check` is
invoked on the ftests directory. This is an undue burden for distros
as it requires uncommon dependencies (lxc, etc.) and can adversely
affect the cgroup sysfs filesystem on the build system. The onus is
on upstream libcgroup to provide a well-tested and vetted *.tar.gz for
the distros that they can then just package up and disseminate. They
shouldn't have to deal with challenging tests and test configurations.
Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com> Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com>
Tom Hromatka [Tue, 8 Apr 2025 22:17:33 +0000 (16:17 -0600)]
github: Delete cgroup v1 workflows
Github has deprecated Ubuntu 20. Since this was the last runner still
running cgroup v1 by default, we no longer have a runner than can run
the automated tests in cgroup v1 mode. Therefore, delete the cgroup v1
automated test runs.
Note that we'll continue to run them by hand periodically and before
releases, but upstream changes may end up breaking/affecting cgroup v1
support. We strongly recommend migrating to cgroup v2.
Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com> Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com>
Tom Hromatka [Tue, 1 Apr 2025 18:35:15 +0000 (18:35 +0000)]
gunit: Promote namespaces in test 001 and 013 to heap variables
On one of my ubuntu22 machines, the NAMESPACE5[] character array was
outputing garbage for the last test. Looks like gtest was running it in
another thread, and the pointer was garbage. Promote the NAMESPACE*
variables to be heap variables so that they're safely available for all
threads.
Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com> Acked-by: Kamalesh Babulal <kamalesh.babulal@oracle.com>
Tom Hromatka [Tue, 1 Apr 2025 17:41:01 +0000 (17:41 +0000)]
ftests: Retry getting pid of sleep process
The ftests framework uses sleeping processes to test various cgroup
features. Sometimes ps throws an error when we try to enumerate
processes via `ps x` within an lxc container.
The error thrown by ps is as follows:
Signal 21 (TTIN) caught by ps (3.3.17).
ps:ps/display.c:70: please report this bug
Fix this by adding a retry loop for the ps command, as it usually works
in a subsequent pass.
Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com> Acked-by: Kamalesh Babulal <kamalesh.babulal@oracle.com>
Tom Hromatka [Thu, 27 Mar 2025 15:28:28 +0000 (15:28 +0000)]
ftests: Fix test 005 on Cgroup v2 Ubuntu22
Test 005-cgsnapshot-basic_snapshot_v2.py was failing on Ubuntu22
(non-sudo tests) because the cpuset.cpus.exclusive.effective field
is no longer being reported by the kernel in that case.
Remove that line from the expected output for the non-sudo tests but
leave it in for the sudo tests as it's still reported there.
Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com> Acked-by: Kamalesh Babulal <kamalesh.babulal@oracle.com>
Now, the process name and destination cgroup are allowed to have spaces
within quotes. Update the man page to reflect the allowed characters
for both fields and add an example demonstrating rule usage.
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
TJH: small formatting changes
Tom Hromatka [Wed, 22 Jan 2025 12:12:59 +0000 (17:42 +0530)]
gunit: Add unit tests for get_next_rule_field()
[----------] 10 tests from GetNextRuleField
[ RUN ] GetNextRuleField.InvalidParameters
[ OK ] GetNextRuleField.InvalidParameters (0 ms)
[ RUN ] GetNextRuleField.FieldLenTooSmall
[ OK ] GetNextRuleField.FieldLenTooSmall (0 ms)
[ RUN ] GetNextRuleField.FieldLenTooSmallWithQuotes
[ OK ] GetNextRuleField.FieldLenTooSmallWithQuotes (0 ms)
[ RUN ] GetNextRuleField.FieldLenTooSmallWithQuotes2
[ OK ] GetNextRuleField.FieldLenTooSmallWithQuotes2 (0 ms)
[ RUN ] GetNextRuleField.UserAndProcess
[ OK ] GetNextRuleField.UserAndProcess (0 ms)
[ RUN ] GetNextRuleField.UserOnly
[ OK ] GetNextRuleField.UserOnly (0 ms)
[ RUN ] GetNextRuleField.KeyWithQuotesAndSpaces
[ OK ] GetNextRuleField.KeyWithQuotesAndSpaces (0 ms)
[ RUN ] GetNextRuleField.DestinationWithQuotes
[ OK ] GetNextRuleField.DestinationWithQuotes (0 ms)
[ RUN ] GetNextRuleField.TabsAsDelimiters
[ OK ] GetNextRuleField.TabsAsDelimiters (0 ms)
[ RUN ] GetNextRuleField.RuleWithOptions
[ OK ] GetNextRuleField.RuleWithOptions (0 ms)
[----------] 10 tests from GetNextRuleField (0 ms total)
Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Kamalesh Babulal [Sun, 26 Jan 2025 06:01:25 +0000 (11:31 +0530)]
config: Fix data race reported in cgroup_config_insert_into_mount_table
Fix the following data race issue reported by Coverity:
CID 465888: (#1 of 1): Check of thread-shared field evades lock
acquisition (LOCK_EVASION):
"The data guarded by this critical section may be read while in an
inconsistent state or modified by multiple racing threads.
In cgroup_config_insert_into_mount_table: Checking the value of a
thread-shared field outside of a locked region to determine if a locked
operation involving that thread shared field has completed."
Fix it by moving the config_table_index value check too under the
config_table_lock.
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Kamalesh Babulal [Sun, 19 Jan 2025 05:48:02 +0000 (11:18 +0530)]
src/config: Fix data race reported by Coverity
Fix the following data race issue reported by Coverity:
CID 465888:Check of thread-shared field evades lock acquisition
(LOCK_EVASION):
"The data guarded by this critical section may be read while in an
inconsistent state or modified by multiple racing threads.
In cgroup_config_insert_into_namespace_table: Checking the value of a
thread-shared field outside of a locked region to determine if a locked
operation involving that thread shared field has completed."
Fix it by moving the namespace_table_index value check too under the
namespace_table_lock
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Aaron Tomlin [Mon, 13 Jan 2025 18:18:05 +0000 (11:18 -0700)]
api: Add extra debugging when matching rule to a group
In the context of a group rule (i.e. indicated by '@' used to prefix the
actual group name), getgrnam(3) is used to provide a pointer to a group
file entry that may contain a NULL-terminated array of pointers to group
members. A user can belong to multiple groups. With this information, we
then check the username that corresponds to the specified UID against
each group member for a match. This patch makes it possible to see this
information if debug level logging is enabled.
Use the new cgroup_get_loglevel() API to optimize the rule loop to
minimize performance impacts.
Kamalesh Babulal [Tue, 17 Dec 2024 05:56:30 +0000 (11:26 +0530)]
ftests/consts.py: Add more cpu controller output (v2)
Upstream v6.13 kernel commit aefa398d93d5 ("cgroup/rstat: Tracking
cgroup-level niced CPU time") added "nice_usec" to the cgroup base stats
cputime, add it to the list of cpu controller (v2) expected output.
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Tom Hromatka [Mon, 13 Jan 2025 17:31:41 +0000 (10:31 -0700)]
github: Ignore unused data errors in lcov
The "Unit Tests" job was throwing the following lcov error:
Summary coverage rate:
lines......: 20.5% (1073 of 5227 lines)
functions..: 29.0% (73 of 252 functions)
branches...: no data found
(use "lcov --ignore-errors unused ..." to bypass this error)
Add "--ignore errors unused" to the lcov command to work around this.
Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Tom Hromatka [Mon, 13 Jan 2025 17:19:34 +0000 (10:19 -0700)]
github: Revert to Ubuntu 22.04 (from latest) for cgroup v2 testing
Cgroup v2 testing on Ubuntu 24.04 is throwing the following errors.
Revert to Ubuntu 22.04 until the issue is resolved in the distro.
/home/runner/work/libcgroup/libcgroup/src/.libs/libcgroup.so.0)
Test:
012-cgget-multiple_r_flags2.py - RunError:
command = ['sudo', 'lxc', 'exec', 'TestLibcg', '--',
'/home/runner/work/libcgroup/libcgroup/src/tools/cgcreate', '-g',
'memory:012cgget1']
ret = 1
stdout =
stderr =
/home/runner/work/libcgroup/libcgroup/src/tools/.libs/cgcreate:
/lib/x86_64-linux-gnu/libc.so.6: version `GLIBC_2.38' not found
(required by
/home/runner/work/libcgroup/libcgroup/src/tools/.libs/cgcreate)
/home/runner/work/libcgroup/libcgroup/src/tools/.libs/cgcreate:
/lib/x86_64-linux-gnu/libc.so.6: version `GLIBC_2.38' not found
(required by
/home/runner/work/libcgroup/libcgroup/src/.libs/libcgroup.so.0)
Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
gcc -o cgrp_get_cgrp_name cgrp_get_cgrp_name.c -lcgroup
/usr/bin/ld: /tmp/ccIyBv5c.o: in function `main':
cgrp_get_cgrp_name.c:(.text+0xaa): undefined reference to `cgroup_get_cgroup_name'
collect2: error: ld returned 1 exit status
The API was introduced by commit ca32d88ef56b1 ("wrappers: Add a
cgroup_get_cgroup_name API") but missed adding it to libcgroup.map file,
causing the linkage issue. Fix the issue by adding the API at the
libcgroup.map under version it was introduced.
Fixes: https://github.com/libcgroup/libcgroup/issues/459 Reported-by: Ron Nazarov <ron@noisytoot.org> Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>