Skip to content

[8.19](backport #51520) osquerybeat: fix data race on osquery runner cancel function#51564

Merged
orestisfl merged 2 commits into
8.19from
mergify/bp/8.19/pr-51520
Jul 1, 2026
Merged

[8.19](backport #51520) osquerybeat: fix data race on osquery runner cancel function#51564
orestisfl merged 2 commits into
8.19from
mergify/bp/8.19/pr-51520

Conversation

@mergify

@mergify mergify Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Proposed commit message

osquerybeat: fix data race on osquery runner cancel function

The `process` closure in `osqueryRunner.Run` reads the `cn` (context cancel
function) variable without holding the mutex, while the goroutine running
osqueryd can concurrently set `cn = nil` through `cancel()`. This is a data
race that can cause a nil-pointer crash when osquery restarts after a
configuration change.

Fix by guarding every read and write of `cn` in `process` with the existing
mutex

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works. Where relevant, I have used the stresstest.sh script to run them under stress conditions and race detector to verify their stability.
  • I have added an entry in ./changelog/fragments using the changelog tool.

How to test this PR locally

script/stresstest.sh --race ./x-pack/osquerybeat/beater '^TestOsqueryRunner'

This is an automatic backport of pull request #51520 done by [Mergify](https://mergify.com).

osquerybeat: fix data race on osquery runner cancel function

The `process` closure in `osqueryRunner.Run` reads the `cn` (context cancel
function) variable without holding the mutex, while the goroutine running
osqueryd can concurrently set `cn = nil` through `cancel()`. This is a data
race that can cause a nil-pointer crash when osquery restarts after a
configuration change.

Fix by guarding every read and write of `cn` in `process` with the existing
mutex

(cherry picked from commit ecb9455)

# Conflicts:
#	x-pack/osquerybeat/beater/osquery_runner.go
@mergify
mergify Bot requested a review from a team as a code owner June 26, 2026 09:47
@mergify mergify Bot added backport conflicts There is a conflict in the backported pull request labels Jun 26, 2026
@mergify
mergify Bot requested a review from a team as a code owner June 26, 2026 09:47
@mergify mergify Bot added backport conflicts There is a conflict in the backported pull request labels Jun 26, 2026
@mergify
mergify Bot requested review from AndersonQ and ebeahan and removed request for a team June 26, 2026 09:47
@mergify

mergify Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

Cherry-pick of ecb9455 has failed:

On branch mergify/bp/8.19/pr-51520
Your branch is up to date with 'origin/8.19'.

You are currently cherry-picking commit ecb945502.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	new file:   changelog/fragments/1782246918-fix-osquerybeat-runner-cancel-data-race.yaml

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   x-pack/osquerybeat/beater/osquery_runner.go

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@botelastic botelastic Bot added the needs_team Indicates that the issue/PR needs a Team:* label label Jun 26, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🤖 GitHub comments

Just comment with:

  • run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)
  • /test : Run the Buildkite pipeline.

@github-actions github-actions Bot added bug Team:Security-Windows Platform Windows Platform Team in Security Solution labels Jun 26, 2026
@botelastic botelastic Bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Jun 26, 2026
@infra-vault-gh-plugin-prod

Copy link
Copy Markdown

Pinging @elastic/sec-windows-platform (Team:Security-Windows Platform)

@github-actions

Copy link
Copy Markdown
Contributor

TL;DR

The agentbeat: Run pre-commit job failed because the backport contains unresolved merge-conflict markers in x-pack/osquerybeat/beater/osquery_runner.go. Resolve the cherry-pick conflict in that file and rerun pre-commit.

Remediation

  • Resolve the conflict in x-pack/osquerybeat/beater/osquery_runner.go by keeping the 8.19 log-level restart behavior while adding the backported mutex protection around all cn reads/writes.
  • Run pre-commit run --all-files before retriggering Buildkite.
Investigation details

Root Cause

This is an unresolved backport/cherry-pick conflict, not an infrastructure failure. The PR diff for x-pack/osquerybeat/beater/osquery_runner.go still includes <<<<<<< HEAD, =======, and >>>>>>> ecb945502 (...) conflict markers around the process closure. The file needs a manual resolution that combines the target branch code with the race fix from the original PR.

Evidence

check for merge conflicts................................................Failed
- hook id: check-merge-conflict
- exit code: 1

x-pack/osquerybeat/beater/osquery_runner.go:66: Merge conflict string '<<<<<<<' found
x-pack/osquerybeat/beater/osquery_runner.go:69: Merge conflict string '=======' found
x-pack/osquerybeat/beater/osquery_runner.go:77: Merge conflict string '>>>>>>>' found
x-pack/osquerybeat/beater/osquery_runner.go:87: Merge conflict string '<<<<<<<' found
x-pack/osquerybeat/beater/osquery_runner.go:91: Merge conflict string '=======' found
x-pack/osquerybeat/beater/osquery_runner.go:93: Merge conflict string '>>>>>>>' found

Verification

  • Not run locally: the local checkout available to this workflow does not contain the PR head conflict markers, but the Buildkite log and PR diff both confirm the failed file and exact markers.

What is this? | From workflow: PR Buildkite Detective

Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.

@mergify

mergify Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

This pull request has not been merged yet. Could you please review and merge it @orestisfl? 🙏

@orestisfl
orestisfl force-pushed the mergify/bp/8.19/pr-51520 branch from e127dfb to e9fd5f6 Compare July 1, 2026 12:14
Resolve merge conflicts from the cherry-pick of ecb9455 by adapting
the race fix to the 8.19 branch: keep the simpler flags-only restart
condition (no logLevel), drop the logLevel variable (not present on
8.19), and use sync.WaitGroup.Go (available since Go 1.24).
@orestisfl
orestisfl force-pushed the mergify/bp/8.19/pr-51520 branch from e9fd5f6 to 147e57a Compare July 1, 2026 12:15
@orestisfl
orestisfl enabled auto-merge (squash) July 1, 2026 12:16
@orestisfl
orestisfl merged commit d2a4ecc into 8.19 Jul 1, 2026
27 of 30 checks passed
@orestisfl
orestisfl deleted the mergify/bp/8.19/pr-51520 branch July 1, 2026 12:54
@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

TL;DR

golangci-lint failed on Linux, Windows, and macOS because gosec flags x-pack/osquerybeat/beater/osquery_runner.go:89 as a G118 context-cancel issue. The immediate fix is to make the stored cancel function visible to the linter, most likely with a targeted //nolint:gosec justification on the context.WithCancel assignment.

Remediation

  • In x-pack/osquerybeat/beater/osquery_runner.go, either refactor so the context.CancelFunc returned by context.WithCancel(parentCtx) is called in a way gosec can prove, or add a narrow suppression on that assignment, e.g. note that cn is invoked through the cancel() helper, which is deferred on Run exit and called on osquery restarts / goroutine exit.
  • Rerun the golangci-lint workflow, or locally run the same linter scope against the PR diff, to confirm the G118 finding is gone.
Investigation details

Root Cause

The backport conflict was resolved by storing the context.WithCancel(parentCtx) cancel function in the outer cn variable, then calling it through the mutex-protected cancel() helper. That is intentional lifecycle management, but gosec does not infer that cn is later invoked by cancel(), so it reports G118 on the assignment.

This is a different failure from the previous detective comment on this PR: that earlier report covered unresolved merge-conflict markers in Buildkite pre-commit, while this workflow fails after conflict resolution in GitHub Actions golangci-lint.

Evidence

x-pack/osquerybeat/beater/osquery_runner.go:89:32: G118: context cancellation function returned by WithCancel/WithTimeout/WithDeadline is not called (gosec)
			ctx, cn = context.WithCancel(parentCtx)
			                            ^
1 issues:
* gosec: 1

Validation

  • Not run locally; this was a read-only CI log investigation. The same G118 finding appears in all three failed OS jobs.

Follow-up

  • If this PR is already merged, apply the remediation in a follow-up PR against 8.19 so future golangci-lint runs are green.

What is this? | From workflow: PR Actions Detective

Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.

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

Labels

backport bug conflicts There is a conflict in the backported pull request Team:Security-Windows Platform Windows Platform Team in Security Solution

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant