Prevent LCS from allocating temp memory over proto-max-bulk-len#9817
Merged
oranagra merged 1 commit intoredis:unstablefrom Nov 21, 2021
oranagra:lcs_mem_limit
Merged
Prevent LCS from allocating temp memory over proto-max-bulk-len#9817oranagra merged 1 commit intoredis:unstablefrom oranagra:lcs_mem_limit
oranagra merged 1 commit intoredis:unstablefrom
oranagra:lcs_mem_limit
Conversation
Member
Author
|
@yossigo should this go on the release notes? |
Collaborator
|
@oranagra I'd say yes, we anyway document the command change so we can mention it also imposes a new memory limit. |
oranagra
added
the
release-notes
yossigo
approved these changes
Nov 21, 2021
hwware
pushed a commit
to hwware/redis
that referenced
this pull request
Dec 20, 2021
…s#9817) LCS can allocate immense amount of memory (sizes of two inputs multiplied by each other). In the past this caused some possible security issues due to overflows, which we solved and also added use of `trymalloc` to return "Insufficient memory" instead of OOM panic zmalloc. But in case overcommit is enabled, it could be that we won't get the OOM panic, and zmalloc will succeed, and then we can get OOM killed by the kernel. The solution here is to prevent LCS from allocating transient memory that's bigger than `proto-max-bulk-len` config. This config is not directly related to transient memory, but using a hard coded value ad well as introducing a specific config seems wrong. This comes to solve an error in the corrupt-dump-fuzzer test that started in the daily CI see redis#9799
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.
LCS can allocate immense amount of memory (sizes of two inputs multiplied by each other).
In the past this caused some possible security issues due to overflows, which we solved
and also added use of
trymallocto return "Insufficient memory" instead of OOM panic zmalloc.But in case overcommit is enabled, it could be that we won't get the OOM panic, and zmalloc
will succeed, and then we can get OOM killed by the kernel.
The solution here is to prevent LCS from allocating transient memory that's bigger than
proto-max-bulk-lenconfig.This config is not directly related to transient memory, but using a hard coded value ad well as
introducing a specific config seems wrong.
This comes to solve an error in the corrupt-dump-fuzzer test that started in the daily CI see #9799