Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a timeout for killing osquery process group #1487

Merged
merged 4 commits into from
Nov 30, 2023

Conversation

RebeccaMahany
Copy link
Contributor

@RebeccaMahany RebeccaMahany commented Nov 30, 2023

Relates to #1442

Impose a timeout on running taskkill on Windows; returns error from running taskkill so that it can be logged by the errgroup. 10 seconds seems like a very safe timeout -- in test, taskkill took around 200 ms.

@RebeccaMahany RebeccaMahany marked this pull request as ready for review November 30, 2023 18:21
Copy link
Contributor

@directionless directionless left a comment

Choose a reason for hiding this comment

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

Actually... Can we log something if the context is canceled? Or maybe that will be in the err anyhow

@RebeccaMahany
Copy link
Contributor Author

RebeccaMahany commented Nov 30, 2023

@directionless we log the returned error here: https://github.com/kolide/launcher/blob/main/pkg/osquery/runtime/runner.go#L434

In test (reducing the timeout to 10ms to force a timeout), that log looks like this for me:

{"caller":"runner.go:437","err":"exit status 1","level":"info","msg":"killing osquery process","session_pid":12832,"ts":"2023-11-30T18:54:27.4700033Z"}

Let me update the error with more information.

@RebeccaMahany
Copy link
Contributor Author

RebeccaMahany commented Nov 30, 2023

@directionless updated, log looks like this in the case of timeout now:

{"caller":"runner.go:437","err":"running taskkill: context err: context deadline exceeded, err: exit status 1","level":"info","msg":"killing osquery process","session_pid":6092,"ts":"2023-11-30T19:12:20.7923216Z"}

And here's what it looks like with a taskkill error:

{"caller":"runner.go:437","err":"running taskkill: output: ERROR: Invalid argument/option - '/QQQ'.\r\nType \"TASKKILL /?\" for usage.\r\n, err: exit status 1","level":"info","msg":"killing osquery process","session_pid":6024,"ts":"2023-11-30T19:17:55.5194436Z"}

@directionless directionless added this pull request to the merge queue Nov 30, 2023
Merged via the queue into kolide:main with commit 21ab35d Nov 30, 2023
26 checks passed
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