fix(driver): avoid kmod crash when a CPU gets enabled at runtime by FedeDP · Pull Request #2252 · falcosecurity/libs

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

 	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

+	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).
Report is 51 commits behind head on master.

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 <nierro92@gmail.com> Co-authored-by: Andrea Terzolo <andreaterzolo3@gmail.com>

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

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

Details

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

Closed

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 <nierro92@gmail.com> Co-authored-by: Andrea Terzolo <andreaterzolo3@gmail.com>

chore(driver): always initialize enum ppm_overlay.

f121b29

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

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

LucaGuerra approved these changes Jan 28, 2025

View reviewed changes

poiana assigned LucaGuerra Jan 28, 2025

poiana merged commit ed95fff into master Jan 28, 2025

poiana deleted the fix/hotplug_kmod branch January 28, 2025 09:43

FedeDP mentioned this pull request Jul 21, 2025

falco 0.40.0 is crashed 139 sometimes falcosecurity/falco#3632

Open

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
poiana merged 2 commits intomasterfrom
fix/hotplug_kmod

FedeDP commented Jan 22, 2025

Uh oh!

FedeDP Jan 22, 2025

Uh oh!

FedeDP Jan 22, 2025

Uh oh!

github-actions Bot commented Jan 22, 2025 •

edited

Loading

Uh oh!

FedeDP commented Jan 22, 2025

Uh oh!

codecov Bot commented Jan 22, 2025 •

edited

Loading

Uh oh!

github-actions Bot commented Jan 22, 2025 •

edited

Loading

Uh oh!

github-actions Bot commented Jan 23, 2025

Uh oh!

FedeDP commented Jan 23, 2025

Uh oh!

FedeDP commented Jan 23, 2025

Uh oh!

Andreagit97 left a comment

Uh oh!

poiana commented Jan 23, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

FedeDP commented Jan 22, 2025

Uh oh!

FedeDP Jan 22, 2025

Choose a reason for hiding this comment

Uh oh!

FedeDP Jan 22, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jan 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Perf diff from master - unit tests

Heap diff from master - unit tests

Heap diff from master - scap file

Benchmarks diff from master

Uh oh!

FedeDP commented Jan 22, 2025

Uh oh!

codecov Bot commented Jan 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions Bot commented Jan 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

X64 kernel testing matrix

ARM64 kernel testing matrix

Uh oh!

github-actions Bot commented Jan 23, 2025

Uh oh!

FedeDP commented Jan 23, 2025

Uh oh!

FedeDP commented Jan 23, 2025

Uh oh!

Andreagit97 left a comment

Choose a reason for hiding this comment

Uh oh!

poiana commented Jan 23, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

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