fix(driver): avoid kmod crash when a CPU gets enabled at runtime #2252

FedeDP · 2025-01-22T14:15:04Z

What type of PR is this?

/kind bug

Any specific area of the project related to this PR?
/area driver-kmod
/area tests

Does this PR require a change in the driver versions?

What this PR does / why we need it:

I also added tests for hotplug in the libscap_test suite.

Which issue(s) this PR fixes:

Fixes #2251

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

fix(driver): avoid kmod crash when a CPU gets enabled at runtime

FedeDP · 2025-01-22T14:16:12Z

driver/main.c

@@ -1820,6 +1812,10 @@ static int record_event_consumer(struct ppm_consumer_t *consumer,
 	ASSERT(ring);

 	ring_info = ring->info;
+	if(!ring_info) {


We won't send any event on the newly enabled CPU; ring_info will be NULL and we would panic.

FedeDP · 2025-01-22T14:17:54Z

driver/main.c

+	struct hotplug_st st;
+	st.sd_action = 1;
+	st.cpu = cpu;
+	return smp_call_on_cpu(0, do_cpu_callback, &st, true);


Run the do_cpu_callback on the CPU 0 to make sure that we are able to send the PPME_CPU_HOTPLUG_E event to userspace.
This is what we do in the bpf probes, since we are guaranteed that CPU 0 is always present.

github-actions · 2025-01-22T14:26:50Z

Perf diff from master - unit tests

    10.75%     -1.03%  [.] sinsp_parser::reset
     5.30%     +0.54%  [.] next_event_from_file
     8.17%     -0.43%  [.] sinsp_evt::get_type
     3.80%     -0.42%  [.] sinsp_evt::load_params
     1.14%     +0.30%  [.] libsinsp::sinsp_suppress::process_event
     0.88%     +0.30%  [.] sinsp_evt::get_syscall_return_value
     0.80%     +0.28%  [.] std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release
     2.89%     -0.26%  [.] sinsp_parser::process_event
     3.05%     -0.25%  [.] sinsp_thread_manager::find_thread
     0.57%     +0.25%  [.] sinsp_fdtable::find

Heap diff from master - unit tests

peak heap memory consumption: 0B
peak RSS (including heaptrack overhead): 0B
total memory leaked: 0B

Heap diff from master - scap file

peak heap memory consumption: 0B
peak RSS (including heaptrack overhead): 0B
total memory leaked: 0B

Benchmarks diff from master

Comparing gbench_data.json to /root/actions-runner/_work/libs/libs/build/gbench_data.json
Benchmark                                                         Time             CPU      Time Old      Time New       CPU Old       CPU New
----------------------------------------------------------------------------------------------------------------------------------------------
BM_sinsp_split_mean                                            +0.0366         +0.0365           144           149           144           149
BM_sinsp_split_median                                          +0.0392         +0.0390           144           150           144           150
BM_sinsp_split_stddev                                          +1.2311         +1.2308             1             2             1             2
BM_sinsp_split_cv                                              +1.1522         +1.1522             0             0             0             0
BM_sinsp_concatenate_paths_relative_path_mean                  -0.0009         -0.0010            57            57            57            57
BM_sinsp_concatenate_paths_relative_path_median                -0.0015         -0.0016            57            57            57            57
BM_sinsp_concatenate_paths_relative_path_stddev                -0.1838         -0.1835             0             0             0             0
BM_sinsp_concatenate_paths_relative_path_cv                    -0.1831         -0.1826             0             0             0             0
BM_sinsp_concatenate_paths_empty_path_mean                     -0.0260         -0.0261            25            24            25            24
BM_sinsp_concatenate_paths_empty_path_median                   -0.0271         -0.0272            25            24            25            24
BM_sinsp_concatenate_paths_empty_path_stddev                   +1.2357         +1.2339             0             0             0             0
BM_sinsp_concatenate_paths_empty_path_cv                       +1.2954         +1.2938             0             0             0             0
BM_sinsp_concatenate_paths_absolute_path_mean                  -0.0042         -0.0043            56            56            56            56
BM_sinsp_concatenate_paths_absolute_path_median                -0.0027         -0.0028            56            56            56            56
BM_sinsp_concatenate_paths_absolute_path_stddev                -0.3322         -0.3324             0             0             0             0
BM_sinsp_concatenate_paths_absolute_path_cv                    -0.3294         -0.3295             0             0             0             0
BM_sinsp_split_container_image_mean                            -0.0111         -0.0112           389           384           389           384
BM_sinsp_split_container_image_median                          -0.0120         -0.0121           389           384           389           384
BM_sinsp_split_container_image_stddev                          -0.5330         -0.5328             3             1             3             1
BM_sinsp_split_container_image_cv                              -0.5278         -0.5275             0             0             0             0

FedeDP · 2025-01-22T14:28:03Z

Drivers CI Build / test-scap-arm64 😆 (bundled_deps) (pull_request) Failing after 2m

Need to make newly introduced tests a little bit less flaky.

codecov · 2025-01-22T14:29:31Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.16%. Comparing base (6c46ed3) to head (f121b29).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #2252   +/-   ##
=======================================
  Coverage   75.16%   75.16%           
=======================================
  Files         278      278           
  Lines       34478    34478           
  Branches     5922     5922           
=======================================
+ Hits        25916    25917    +1     
+ Misses       8562     8561    -1

Flag	Coverage Δ
libsinsp	`75.16% <ø> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

github-actions · 2025-01-22T15:30:10Z

X64 kernel testing matrix

KERNEL	CMAKE-CONFIGURE	KMOD BUILD	KMOD SCAP-OPEN	BPF-PROBE BUILD	BPF-PROBE SCAP-OPEN	MODERN-BPF SCAP-OPEN
amazonlinux2-4.19	🟢	🟢	🟢	🟢	🟢	🟡
amazonlinux2-5.10	🟢	🟢	🟢	🟢	🟢	🟢
amazonlinux2-5.15	🟢	🟢	🟢	🟢	🟢	🟢
amazonlinux2-5.4	🟢	🟢	🟢	🟢	🟢	🟡
amazonlinux2022-5.15	🟢	🟢	🟢	🟢	🟢	🟢
amazonlinux2023-6.1	🟢	🟢	🟢	🟢	🟢	🟢
archlinux-6.0	🟢	🟢	🟢	🟢	🟢	🟢
archlinux-6.7	🟢	🟢	🟢	🟢	🟢	🟢
centos-3.10	🟢	🟢	🟢	🟡	🟡	🟡
centos-4.18	🟢	🟢	🟢	🟢	🟢	🟢
centos-5.14	🟢	🟢	🟢	🟢	🟢	🟢
fedora-5.17	🟢	🟢	🟢	🟢	🟢	🟢
fedora-5.8	🟢	🟢	🟢	🟢	🟢	🟢
fedora-6.2	🟢	🟢	🟢	🟢	🟢	🟢
oraclelinux-3.10	🟢	🟢	🟢	🟡	🟡	🟡
oraclelinux-4.14	🟢	🟢	🟢	🟢	🟢	🟡
oraclelinux-5.15	🟢	🟢	🟢	🟢	🟢	🟢
oraclelinux-5.4	🟢	🟢	🟢	🟢	🟢	🟡
ubuntu-4.15	🟢	🟢	🟢	🟢	🟢	🟡
ubuntu-5.8	🟢	🟢	🟢	🟢	🟢	🟡
ubuntu-6.5	🟢	🟢	🟢	🟢	🟢	🟢

ARM64 kernel testing matrix

KERNEL	CMAKE-CONFIGURE	KMOD BUILD	KMOD SCAP-OPEN	BPF-PROBE BUILD	BPF-PROBE SCAP-OPEN	MODERN-BPF SCAP-OPEN
amazonlinux2-5.4	🟢	🟢	🟢	🟢	🟢	🟡
amazonlinux2022-5.15	🟢	🟢	🟢	🟢	🟢	🟢
fedora-6.2	🟢	🟢	🟢	🟢	🟢	🟢
oraclelinux-4.14	🟢	🟢	🟢	🟡	🟡	🟡
oraclelinux-5.15	🟢	🟢	🟢	🟢	🟢	🟢
ubuntu-6.5	🟢	🟢	🟢	🟢	🟢	🟢

github-actions · 2025-01-23T08:47:52Z

Please double check driver/SCHEMA_VERSION file. See versioning.

/hold

… is hotplugged. Signed-off-by: Federico Di Pierro <[email protected]> Co-authored-by: Andrea Terzolo <[email protected]>

Signed-off-by: Federico Di Pierro <[email protected]>

FedeDP · 2025-01-23T09:12:16Z

/milestone next-driver

FedeDP · 2025-01-23T09:15:59Z

/unhold

Andreagit97

/approve

poiana · 2025-01-23T09:21:41Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Andreagit97, FedeDP

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~driver/OWNERS~~ [Andreagit97,FedeDP]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

poiana added kind/bug Something isn't working release-note dco-signoff: yes area/driver-kmod area/tests approved labels Jan 22, 2025

poiana requested review from hbrueckner and jasondellaluce January 22, 2025 14:15

poiana added the size/L label Jan 22, 2025

FedeDP commented Jan 22, 2025

View reviewed changes

FedeDP mentioned this pull request Jan 22, 2025

NULL pointer dereference in kmod driver while hot plugging a CPU #2251

Open

FedeDP force-pushed the fix/hotplug_kmod branch from b9fdc85 to 8aa9e04 Compare January 22, 2025 14:32

FedeDP force-pushed the fix/hotplug_kmod branch from b5a9c14 to 8d73821 Compare January 22, 2025 16:01

Andreagit97 force-pushed the fix/hotplug_kmod branch from 8d73821 to a0e8b4b Compare January 22, 2025 18:11

FedeDP force-pushed the fix/hotplug_kmod branch from e3c963a to 95cc9b7 Compare January 23, 2025 07:06

poiana added size/XL and removed size/L labels Jan 23, 2025

FedeDP force-pushed the fix/hotplug_kmod branch 3 times, most recently from e5765a8 to 60cacc4 Compare January 23, 2025 08:09

poiana added the do-not-merge/hold label Jan 23, 2025

FedeDP and others added 2 commits January 23, 2025 10:03

fix(driver): avoid crashing when an offline CPU prior to agent start,…

c09b3b6

… is hotplugged. Signed-off-by: Federico Di Pierro <[email protected]> Co-authored-by: Andrea Terzolo <[email protected]>

chore(driver): always initialize enum ppm_overlay.

f121b29

Signed-off-by: Federico Di Pierro <[email protected]>

FedeDP force-pushed the fix/hotplug_kmod branch from fb31889 to f121b29 Compare January 23, 2025 09:03

poiana added size/L and removed size/XL labels Jan 23, 2025

poiana added this to the next-driver milestone Jan 23, 2025

poiana removed the do-not-merge/hold label Jan 23, 2025

Andreagit97 approved these changes Jan 23, 2025

View reviewed changes

poiana assigned Andreagit97 Jan 23, 2025

poiana added the lgtm label Jan 23, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(driver): avoid kmod crash when a CPU gets enabled at runtime #2252

fix(driver): avoid kmod crash when a CPU gets enabled at runtime #2252

FedeDP commented Jan 22, 2025

FedeDP Jan 22, 2025

FedeDP Jan 22, 2025

github-actions bot commented Jan 22, 2025 •

edited

Loading

FedeDP commented Jan 22, 2025

codecov bot commented Jan 22, 2025 •

edited

Loading

github-actions bot commented Jan 22, 2025 •

edited

Loading

github-actions bot commented Jan 23, 2025

FedeDP commented Jan 23, 2025

FedeDP commented Jan 23, 2025

Andreagit97 left a comment

poiana commented Jan 23, 2025

fix(driver): avoid kmod crash when a CPU gets enabled at runtime #2252

Are you sure you want to change the base?

fix(driver): avoid kmod crash when a CPU gets enabled at runtime #2252

Conversation

FedeDP commented Jan 22, 2025

FedeDP Jan 22, 2025

Choose a reason for hiding this comment

FedeDP Jan 22, 2025

Choose a reason for hiding this comment

github-actions bot commented Jan 22, 2025 • edited Loading

Perf diff from master - unit tests

Heap diff from master - unit tests

Heap diff from master - scap file

Benchmarks diff from master

FedeDP commented Jan 22, 2025

codecov bot commented Jan 22, 2025 • edited Loading

Codecov Report

github-actions bot commented Jan 22, 2025 • edited Loading

X64 kernel testing matrix

ARM64 kernel testing matrix

github-actions bot commented Jan 23, 2025

FedeDP commented Jan 23, 2025

FedeDP commented Jan 23, 2025

Andreagit97 left a comment

Choose a reason for hiding this comment

poiana commented Jan 23, 2025

github-actions bot commented Jan 22, 2025 •

edited

Loading

codecov bot commented Jan 22, 2025 •

edited

Loading

github-actions bot commented Jan 22, 2025 •

edited

Loading