-
Notifications
You must be signed in to change notification settings - Fork 1.7k
out_es: Prevent a SIGSEGV when ctx->index is NULL #10353
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
base: master
Are you sure you want to change the base?
out_es: Prevent a SIGSEGV when ctx->index is NULL #10353
Conversation
d3257c8
to
97f4915
Compare
configuration: (this will fail) service:
flush: 1
daemon: off
pipeline:
inputs:
- name: forward
unix_path: /var/run/fluent.sock
tag: log
outputs:
- name: es
match: *
host: valid_host
port: valid_port
index:
suppress_type_name: true
trace_error: true
tls: true configuration: (this will work) service:
flush: 1
daemon: off
pipeline:
inputs:
- name: forward
unix_path: /var/run/fluent.sock
tag: log
outputs:
- name: es
match: *
host: valid_host
port: valid_port
index: any-index
suppress_type_name: true
trace_error: true
tls: true |
Debug log with failing configuration:
|
Log from successful configuration:
|
valgrind:
|
Probably worth adding a simple unit test to exercise the check and also prevent future regressions. Does the opensearch output have the same issue? If so we should fix it too |
Yeah looks like opensearch needs the fix too. For some reason the run code analysis script is causing the out_elastic tests to be skipped, but it is running the opensearch ones (and the code is the same between them on this case). Running the test suite one last time to validate the change and then i'll push and fix the commit message as well. |
If `index` field is blank in the configuration, and `Logstash_Format` and `Generate_ID` are both `off`, a SIGSEGV can occur when trying to generate the index for a set of messages. Signed-off-by: Todd Kennedy <[email protected]>
0c81ebd
to
8705833
Compare
@patrick-stephens the fix for opensearch is in #10365. Do you want me to replicate the artifacts (output logs, configuration, etc) in that PR as well, or is the similarity enough in the platforms/tests? |
This indicates that some of the part of Fluent Bit should cause memory leaks. |
For completeness I would replicate or at least link but I think you've done that already? Main thing is if we look at the PR in isolation is it complete enough. |
@cosmo0920 I did not since my change causes fluent-bit to exit early and allocates no objects nor changes any data or structures. it seems if there are leaks this change would not have introduced them (unless there was something else I was supposed to call instead of however, here is the log from that.
|
If
index
field is blank in the configuration, andLogstash_Format
andGenerate_ID
are bothoff
, a SIGSEGV can occur when trying to generate the index for a set of messages.Fixes #10339
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-test
label 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.