-
Notifications
You must be signed in to change notification settings - Fork 7.8k
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
zlog: fix free, fpm_conf: set NULL after free #8785
Conversation
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.
Please can you create 2 PR's for this. One for conf and another one for zlog.
sapi/fpm/fpm/fpm_conf.c
Outdated
if (ini_include) { | ||
free(ini_include); | ||
ini_include = NULL; | ||
} |
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.
This looks like a correct and complete fix. Nice catch.
@@ -645,7 +645,7 @@ zlog_bool zlog_stream_set_msg_suffix( | |||
stream->msg_final_suffix_len = strlen(final_suffix); | |||
len = stream->msg_final_suffix_len + 1; | |||
if (stream->msg_final_suffix != NULL) { | |||
free(stream->msg_suffix); | |||
free(stream->msg_final_suffix); |
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.
This is even better catch. It's correct for the only used call of this function and I think it might be potentially reason for #8517 although not 100% sure about it. Anyway we should definitely fix this. It might need to be slightly changed logic (possibly later) as that function has got some logic that if you pass suffix as well as final_suffix, then final suffix is part of allocated suffix (see above) - that's the actual reason of this bug being introduced. However that path is not used so it will always be allocated below.
2nd PR is #8794 |
Can you squash it to a single commit pls? |
Revert "zlog: fix free“. will be in another PR This reverts commit 9fccc56.
Fix already applied 325ca31 |
No description provided.