-
Notifications
You must be signed in to change notification settings - Fork 1.8k
in_forward: fix connection release on pause memory corruption [backport to 4.1] #11179
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
Conversation
This change fixes a use after free issue related to connection disposal which caused the event handler to access invalid memory when the memory limits were exceeded during ingestion. In order to overcome this issue we track the plugin instances state and delay the connection cleanup process. Signed-off-by: Leonardo Albertovich <leonardo@alminana.com.ar>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| ctx->state = state_backup; | ||
|
|
||
| if (ctx->state == FW_INSTANCE_STATE_PAUSED) { | ||
| fw_conn_del_all(ctx); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Preserve paused state when accepting connections
During connection accept, in_fw_collect unconditionally restores ctx->state to the prior value at exit and immediately checks that restored value to decide whether to purge connections. If in_fw_pause sets ctx->state to FW_INSTANCE_STATE_PAUSED while a client is being accepted, this restore overwrites the paused state with the old RUNNING value and the subsequent fw_conn_del_all guard never fires, so the paused transition is lost and the just-accepted connection keeps processing traffic even though ingestion was paused to relieve backpressure. The state reset here should only occur when the state was not changed externally.
Useful? React with 👍 / 👎.
backport of #11114
(my first backport so plz let me know if Ive missed something or if I need to backport to more branches. I wasn't sure whether to do 4.1 or 4.2, or both (?)).
This change fixes a use after free issue related to connection disposal which caused the event handler to access invalid memory when the memory limits were exceeded during ingestion.
In order to overcome this issue we track the plugin instances state and delay the connection cleanup process.
Enter
[N/A]in the box, if an item is not applicable to your change.Testing
Before we can approve your change; please submit the following in a comment:
If this is a change to packaging of containers or native binaries then please confirm it works for all targets.
ok-package-testlabel to test for all targets (requires maintainer to do).Documentation
Backporting
Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.