Fix use after free on blocking RM_Call.#12342
Merged
oranagra merged 4 commits intoredis:unstablefrom Jun 25, 2023
Merged
Conversation
blocking RM_Call was introduced on: https://github.com/redis/redis/pull/11568It allows a module to perform blocking commands and get the reply asynchronously.If the command gets block, a special promise CallReply is returned that allow to setthe unblock handler. The unblock hander will be called when the command invocation finishand it gets, as input, the command real reply. The issue was that the real CallReply was created using a stack allocated RedisModuleCtxwhich is no longer available after the unblock handler finishes. So if the module keepsthe CallReply after the unblock handler finished, the CallReply holds a pointer to invalidmemory and will try to access it when the CallReply will be released. The solution is to create the CallReply with a NULL context to make it totally detachedand can be freed freely when the module wants. Test was added to cover this case, running the test with valgrind before the fix shows the use after free error. With the fix, there are not valgrind errors.
oranagra
approved these changes
Jun 25, 2023
Co-authored-by: Oran Agra <[email protected]>
oranagra
added
the
release-notes
Merged
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
blocking RM_Call was introduced on: #11568 (7.2 RC1), which allows a module to perform blocking commands and get the reply asynchronously.If the command gets block, a special promise CallReply is returned that allow to set the unblock handler. The unblock handler will be called when the command invocation finish and it gets, as input, the command real reply.
The issue was that the real CallReply was created using a stack allocated RedisModuleCtx which is no longer available after the unblock handler finishes. So if the module keeps the CallReply after the unblock handler finished, the CallReply holds a pointer to invalid memory and will try to access it when the CallReply will be released.
The solution is to create the CallReply with a NULL context to make it totally detached and can be freed freely when the module wants.
Test was added to cover this case, running the test with valgrind before the fix shows the use after free error. With the fix, there are no valgrind errors.
unrelated: adding a missing
$rd closein many tests in that file.