Skip to content

Update Falco with event parsing fix - #3681

Open
Stringy wants to merge 2 commits into
masterfrom
giles/fix-sinsp-event-parse-crash
Open

Update Falco with event parsing fix#3681
Stringy wants to merge 2 commits into
masterfrom
giles/fix-sinsp-event-parse-crash

Conversation

@Stringy

@Stringy Stringy commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

Description

Updates the falcosecurity-libs submodule to include the lenient filename parameter extraction fix.

stackrox/falcosecurity-libs#98

Checklist

  • Investigated and inspected CI test results
  • Updated documentation accordingly

Automated testing

  • Added unit tests
  • Added integration tests
  • Added regression tests

If any of these don't apply, please comment below.

Testing Performed

CI shuold be enough. Fedora CoreOS x86 was crashing, so this is the priority.

@Stringy
Stringy requested a review from a team as a code owner July 31, 2026 09:44
@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • Chores
    • Updated the bundled Falco security libraries to a newer revision.
    • No user-facing functionality or public interfaces changed.

Walkthrough

The falcosecurity-libs submodule reference changes from commit fdb27236a5b589fb2f658cb3453b6e52388eb22c to commit 713d313c67b9ce66690f39e31b9ec2e936eb0ccd.

Changes

Falco libraries update

Layer / File(s) Summary
Update submodule reference
falcosecurity-libs
The submodule reference changes to commit 713d313c67b9ce66690f39e31b9ec2e936eb0ccd.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Suggested reviewers: erthalion

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description check ✅ Passed The description includes the required sections, explains the submodule update and crash issue, and states the testing rationale.
Title check ✅ Passed The title clearly identifies the Falco update and the event parsing fix addressed by the submodule change.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch giles/fix-sinsp-event-parse-crash

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@collector/lib/system-inspector/Service.cpp`:
- Around line 130-136: Update the sinsp_exception handling in GetNext() so only
genuinely recoverable parse failures return nullptr for Run() to retry;
propagate fatal or invalid-state exceptions or reinitialize the capture instead.
Replace the unthrottled CLOG warning with CLOG_THROTTLED for repeated failures,
preserving the existing successful-event path.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 50f110a2-060a-44bb-857d-5889c44504de

📥 Commits

Reviewing files that changed from the base of the PR and between 2b7522e and 7219d7e.

📒 Files selected for processing (2)
  • collector/lib/system-inspector/Service.cpp
  • falcosecurity-libs

Comment thread collector/lib/system-inspector/Service.cpp Outdated
@codecov-commenter

codecov-commenter commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 27.43%. Comparing base (2b7522e) to head (95dfff9).
⚠️ Report is 19 commits behind head on master.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3681   +/-   ##
=======================================
  Coverage   27.43%   27.43%           
=======================================
  Files          94       94           
  Lines        5406     5406           
  Branches     2535     2535           
=======================================
  Hits         1483     1483           
  Misses       3198     3198           
  Partials      725      725           
Flag Coverage Δ
collector-unit-tests 27.43% <ø> (ø)

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

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

@Stringy
Stringy force-pushed the giles/fix-sinsp-event-parse-crash branch from 7219d7e to 701263d Compare July 31, 2026 10:30
@Stringy Stringy changed the title Catch sinsp_exception in Service::GetNext() Update Falco with event parsing fix Jul 31, 2026
Updates the falcosecurity-libs submodule to include a fix for
mismatched event parameter lengths that caused collector to crash
via sinsp_exception -> std::unexpected() -> abort().

The string extraction templates now tolerate parameter lengths that
don't match the null-terminated string length, which has been
observed on some kernels for both string and integer parameters.
@Stringy
Stringy force-pushed the giles/fix-sinsp-event-parse-crash branch from 701263d to 24427d1 Compare August 5, 2026 12:13
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

Update submodule to include enhanced diagnostic logging in
throw_invalid_len_error(). This reverts the param_len-1 workaround
so the crash still occurs on Fedora CoreOS CI, but now with full
raw event dumps (header fields, length array, data region) to
identify the root cause of the parameter corruption.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@falcosecurity-libs`:
- Line 1: Use a revision that includes the lenient filename extraction fix and
passes the Fedora CoreOS x86 reproducer; do not use revision
713d313c67b9ce66690f39e31b9ec2e936eb0ccd, which only restores throwing behavior
and diagnostics. Ensure diagnostic handling bounds all raw event access by
raw->len, even when raw->nparams is corrupted.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 0c5f8578-f7ec-4852-964f-3c00ae7adde8

📥 Commits

Reviewing files that changed from the base of the PR and between 24427d1 and 95dfff9.

📒 Files selected for processing (1)
  • falcosecurity-libs

Comment thread falcosecurity-libs
@@ -1 +1 @@
Subproject commit fdb27236a5b589fb2f658cb3453b6e52388eb22c
Subproject commit 713d313c67b9ce66690f39e31b9ec2e936eb0ccd

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

module="falcosecurity-libs"
old="fdb27236a5b589fb2f658cb3453b6e52388eb22c"
new="713d313c67b9ce66690f39e31b9ec2e936eb0ccd"

git submodule update --init "$module"
git -C "$module" fetch --quiet origin "$old" "$new"
git -C "$module" diff --stat "$old" "$new"
git -C "$module" diff --unified=40 "$old" "$new" -- |
  rg -n -i 'param_len|filename|lenient|raw event|dump|log' || true

Repository: stackrox/collector

Length of output: 1854


🏁 Script executed:

#!/bin/bash
set -euo pipefail

module="falcosecurity-libs"
old="fdb27236a5b589fb2f658cb3453b6e52388eb22c"
new="713d313c67b9ce66690f39e31b9ec2e936eb0ccd"

printf '%s\n' '--- commit metadata ---'
git -C "$module" show -s --format='%H%n%P%n%s%n%b' "$new"

printf '%s\n' '--- complete changed-file summary ---'
git -C "$module" diff --name-status "$old" "$new"
git -C "$module" diff --stat "$old" "$new"

printf '%s\n' '--- complete event.cpp diff ---'
git -C "$module" diff --unified=12 "$old" "$new" -- userspace/libsinsp/event.cpp

printf '%s\n' '--- relevant symbols and callers ---'
rg -n -C 4 'param_len|lenient|filename|buffer_to_multiline_hex|raw event|parameter raw data' "$module/userspace" "$module/test" "$module/tests" 2>/dev/null || true

Repository: stackrox/collector

Length of output: 50375


🏁 Script executed:

#!/bin/bash
set -euo pipefail

module="falcosecurity-libs"
old="fdb27236a5b589fb2f658cb3453b6e52388eb22c"
new="713d313c67b9ce66690f39e31b9ec2e936eb0ccd"

printf '%s\n' '--- changed files ---'
git -C "$module" diff --name-status "$old" "$new"

printf '%s\n' '--- commit metadata ---'
git -C "$module" show -s --format='%H%n%P%n%s%n%b' "$new"

printf '%s\n' '--- focused diff ---'
git -C "$module" diff --unified=8 "$old" "$new" -- userspace/libsinsp/event.cpp

printf '%s\n' '--- parent and target implementations ---'
for rev in "$old" "$new"; do
  echo "[$rev]"
  git -C "$module" show "$rev:userspace/libsinsp/event.cpp" |
    rg -n -C 12 'parameter raw data|param_len|raw header\+lengths|param data region|buffer_to_multiline_hex'
done

Repository: stackrox/collector

Length of output: 21854


🏁 Script executed:

#!/bin/bash
set -euo pipefail

module="falcosecurity-libs"

printf '%s\n' '--- event and parameter layout definitions ---'
rg -n -C 8 'struct scap_evt|typedef struct scap_evt|struct ppm_evt_hdr|PPM_MAX_EVENT_PARAMS|EF_LARGE_PAYLOAD' \
  "$module" -g '*.h' -g '*.c' -g '*.cpp' | head -n 240

printf '%s\n' '--- logger severity and sink behavior ---'
rg -n -C 8 'void .*log|log\\(const|SEV_ERROR|set_severity|add_file_log|m_logger' \
  "$module/userspace/libsinsp" -g '*.h' -g '*.cpp' | head -n 260

printf '%s\n' '--- raw-event bounds checks near consumers ---'
rg -n -C 10 'raw->len|m_evt->get_scap_evt|len_array|buffer_to_multiline_hex' \
  "$module/userspace/libsinsp" -g '*.cpp' -g '*.h' | head -n 260

Repository: stackrox/collector

Length of output: 23413


🏁 Script executed:

#!/bin/bash
set -euo pipefail

module="falcosecurity-libs"

printf '%s\n' '--- event header definitions ---'
rg -n -C 12 'ppm_evt_hdr' "$module" -g '*.h' -g '*.c' -g '*.cpp' | head -n 220

printf '%s\n' '--- logger API and severity behavior ---'
rg -n -C 10 'SEV_ERROR|add_file_log|void log|log\\(' "$module/userspace/libsinsp" -g '*.h' -g '*.cpp' | head -n 260

printf '%s\n' '--- event length validation ---'
rg -n -C 12 'event_len|evt->len|e->len|len < sizeof|len >|SCAP_MAX' \
  "$module/userspace/libscap" "$module/userspace/libsinsp" -g '*.c' -g '*.cpp' -g '*.h' | head -n 260

Repository: stackrox/collector

Length of output: 15852


🏁 Script executed:

#!/bin/bash
set -euo pipefail

module="falcosecurity-libs"

printf '%s\n' '--- target diagnostic block with line numbers ---'
git -C "$module" show --format= --no-ext-diff \
  713d313c67b9ce66690f39e31b9ec2e936eb0ccd -- userspace/libsinsp/event.cpp |
  sed -n '1,130p'

printf '%s\n' '--- get_scap_evt and event storage ---'
rg -n -C 10 'get_scap_evt|m_pevt|scap_evt \*|scap_evt\*|m_evt' \
  "$module/userspace/libsinsp" -g '*.h' -g '*.cpp' | head -n 280

printf '%s\n' '--- allocation and capture-buffer bounds ---'
rg -n -C 10 'evt->len|event->len|m_evt.*len|SCAP_MAX|buffer.*size|next.*event|scap_evt' \
  "$module/userspace/libscap" "$module/userspace/libsinsp" -g '*.c' -g '*.cpp' -g '*.h' |
  head -n 320

Repository: stackrox/collector

Length of output: 50374


Treat this revision as diagnostic only.

713d313c67b9ce66690f39e31b9ec2e936eb0ccd restores the throwing behavior and adds diagnostics only. It does not contain the lenient filename extraction fix. Its diagnostic path logs raw event data and can read beyond raw->len when raw->nparams is corrupted. Use a revision with the extraction fix and a passing Fedora CoreOS x86 reproducer.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@falcosecurity-libs` at line 1, Use a revision that includes the lenient
filename extraction fix and passes the Fedora CoreOS x86 reproducer; do not use
revision 713d313c67b9ce66690f39e31b9ec2e936eb0ccd, which only restores throwing
behavior and diagnostics. Ensure diagnostic handling bounds all raw event access
by raw->len, even when raw->nparams is corrupted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants