-
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.0] #11182
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.
Prevent overwriting paused state after accepting a client
The collector backs up the plugin state, switches it to FW_INSTANCE_STATE_ACCEPTING_CLIENT, and at the end unconditionally restores the backup before checking for a pause. If in_fw_pause() fires while a connection is being accepted it sets ctx->state to FW_INSTANCE_STATE_PAUSED so it can close connections later, but these lines overwrite that pause state back to the old value and the subsequent fw_conn_del_all(ctx) guard never runs. As a result existing connections remain active and continue ingesting while the input is paused, defeating backpressure. The state should only be restored when it is still ACCEPTING_CLIENT; if the state has become PAUSED we should keep that state and trigger cleanup similar to fw_conn_event.
Useful? React with 👍 / 👎.
Backporting of #11114.
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.