logproto: fix invalid buffer handling when encoding() is used #3892
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Build SUCCESS |
MrAnno
force-pushed
the
fix-file-enc-buf-split
branch
from
ee4f700
to
26332c7
Compare
MrAnno
force-pushed
the
fix-file-enc-buf-split
branch
from
26332c7
to
f7e2226
Compare
|
@kira-syslogng do stresstest |
|
Build SUCCESS |
|
Kira-stress-test: Build SUCCESS |
|
@Kokan I need to validate my theory, but this might be one of the root causes of the Big5 encoding issue that we've discussed IRL. |
bazsi
reviewed
Jan 28, 2022
bazsi
reviewed
Jan 29, 2022
bazsi
reviewed
Jan 29, 2022
bazsi
reviewed
Jan 29, 2022
MrAnno
force-pushed
the
fix-file-enc-buf-split
branch
from
f7e2226
to
f06ad77
Compare
MrAnno
force-pushed
the
fix-file-enc-buf-split
branch
from
f06ad77
to
e02546a
Compare
|
@kira-syslogng do stresstest |
|
Build SUCCESS |
|
Kira-stress-test: Build SUCCESS |
bazsi
reviewed
Jan 30, 2022
Signed-off-by: László Várady <laszlo.varady@protonmail.com>
MrAnno
force-pushed
the
fix-file-enc-buf-split
branch
2 times, most recently
from
1c36b11
to
cd59631
Compare
|
Build FAILURE |
MrAnno
force-pushed
the
fix-file-enc-buf-split
branch
from
cd59631
to
142de5a
Compare
|
Build SUCCESS |
MrAnno
force-pushed
the
fix-file-enc-buf-split
branch
2 times, most recently
from
7115c34
to
f9d1081
Compare
|
@kira-syslogng do stresstest |
bazsi
reviewed
Jan 30, 2022
|
Build SUCCESS |
|
Kira-stress-test: Build SUCCESS |
bazsi
previously approved these changes
Jan 31, 2022
Kokan
previously approved these changes
Jan 31, 2022
|
We're running some additional tests before merge. |
gaborznagy
reviewed
Feb 1, 2022
This test reproduces a bug in log_proto_text_server_split_buffer(), where an invalid (partially overwritten) buffer slice is passed to our decoder function (log_proto_text_server_get_raw_size_of_buffer()). The subsequent commit fixes this bug. Signed-off-by: László Várady <laszlo.varady@protonmail.com>
log_proto_text_server_split_buffer() moves data within self->data for which we maintain additional pointers: buffer_start. This pointer needs to be updated after the memmove() call, otherwise it could point to partially overwritten data in case the memmove() operation overlaps. A few lines below, buffer_start is passed to a decoder function called log_proto_text_server_get_raw_size_of_buffer(), which could fail and report incorrect positions for file sources. This could result in message duplication, partial message loss, but it only occurs when non-fixed length or less known fixed-length encodings are used (encodings that are not listed in fixed_encodings). Signed-off-by: László Várady <laszlo.varady@protonmail.com>
Signed-off-by: László Várady <laszlo.varady@protonmail.com>
Signed-off-by: László Várady <laszlo.varady@protonmail.com>
split_buffer() moves data from the middle/end of the buffer to the beginning, which is the responsibility of LogProtoBufferedServer. Currently, the split logic is not called when the end of the buffer is reached, it is called on specific LogProtoTextServer events, so the method is currently "protected" instead of "private". Signed-off-by: László Várady <laszlo.varady@protonmail.com>
Signed-off-by: László Várady <laszlo.varady@protonmail.com>
Signed-off-by: László Várady <laszlo.varady@protonmail.com>
Signed-off-by: László Várady <laszlo.varady@protonmail.com>
Signed-off-by: László Várady <laszlo.varady@protonmail.com>
Signed-off-by: László Várady <laszlo.varady@protonmail.com>
Signed-off-by: László Várady <laszlo.varady@protonmail.com>
MrAnno
force-pushed
the
fix-file-enc-buf-split
branch
from
f9d1081
to
46bfe6c
Compare
|
Build SUCCESS |
gaborznagy
approved these changes
Feb 2, 2022
bazsi
approved these changes
Feb 3, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
log_proto_text_server_split_buffer()moves data withinself->datafor which we maintain additional pointers:buffer_start.This pointer needs to be updated after the
memmove()call, otherwise it could point to partially overwritten data in case thememmove()operation overlaps.A few lines below my fix,
buffer_startis passed to a decoder function calledlog_proto_text_server_get_raw_size_of_buffer(), which could fail and report incorrect positions.This could result in message duplication, partial message loss, but it only occurs when non-fixed length or less known fixed-length encodings are used (encodings that are not listed in
fixed_encodings).