Skip to content

Fix GH-16628: FPM logs are getting corrupted with this log statement #16680

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

Closed
wants to merge 1 commit into from

Conversation

nielsdos
Copy link
Member

@nielsdos nielsdos commented Nov 2, 2024

zlog_buf_prefix() can return a larger length than what actually was written due to its use of snprintf(). The code in
zlog_stream_prefix_ex() does not take this into account, other callers do. What ends up happening then is that stream->length is set to the length as if snprintf() was able to write all bytes, causing stream->length to become larger than stream->buf.size, causing a segfault.

In case the buffer was too small we try with a larger buffer up to a limit of zlog_limit. This makes sure that the stream length will remain bounded by the buffer size.

This also adds assertions to make the programmer intent clear and catch this more easily in debug builds.

zlog_buf_prefix() can return a larger length than what actually was
written due to its use of snprintf(). The code in
zlog_stream_prefix_ex() does not take this into account, other callers
do. What ends up happening then is that stream->length is set to the
length as if snprintf() was able to write all bytes, causing
stream->length to become larger than stream->buf.size, causing a
segfault.

In case the buffer was too small we try with a larger buffer up to a
limit of zlog_limit. This makes sure that the stream length will remain
bounded by the buffer size.

This also adds assertions to make the programmer intent clear and catch
this more easily in debug builds.
Copy link
Member

@bukka bukka left a comment

Choose a reason for hiding this comment

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

Nice catch. I think I must have thought that snprintf returns number of bytes actually written. I guess it's a common mistake with snprintf... The changes look good to me. Thanks.

@nielsdos nielsdos closed this in e643129 Nov 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FPM logs are getting corrupted with this log statement.
2 participants