Skip to content

Fix memory leak in streamGetEdgeID#10753

Merged
oranagra merged 1 commit intoredis:unstablefrom
Yuuoniy:fix-memleak-streamGetEdgeID
May 22, 2022
Merged

Fix memory leak in streamGetEdgeID#10753
oranagra merged 1 commit intoredis:unstablefrom
Yuuoniy:fix-memleak-streamGetEdgeID

Conversation

@Yuuoniy
Copy link
Contributor

@Yuuoniy Yuuoniy commented May 19, 2022

si is initialized by streamIteratorStart(), we should call streamIteratorStop() on it when done. #10752

si is initialized by streamIteratorStart(), we should call
streamIteratorStop() on it when done.
@enjoy-binbin enjoy-binbin added the state:to-be-merged The PR should be merged soon, even if not yet ready, this is used so that it won't be forgotten label May 19, 2022
@oranagra oranagra linked an issue May 22, 2022 that may be closed by this pull request
@oranagra
Copy link
Member

@Yuuoniy thank you.
p.s. the reason why this doesn't usually leak is that rax iterator has static buffers for short keys and shallow stacks.
for streams, the keys are always short, so the key would never leak any memory.
but the stack may leak on large streams.

this looks like a regression in redis 7.0 (#9127) that could in cause leaks in trimming (XADD, XTRIM), and also XDEL and AOFRW.

@itamarhaber @guybe7 correct me if i'm wrong.

@oranagra oranagra merged commit 4a7a4e4 into redis:unstable May 22, 2022
@oranagra oranagra added the release-notes indication that this issue needs to be mentioned in the release notes label May 22, 2022
@Yuuoniy
Copy link
Contributor Author

Yuuoniy commented May 22, 2022

@oranagra I see, thanks for your explanation. I found this one when went through the PR #10353 , and thought it will result in memory leak as well.

@oranagra oranagra mentioned this pull request Jun 8, 2022
@oranagra
Copy link
Member

Apparently there's a CVE for this issue: GHSA-35rf-7vhx-9phr

enjoy-binbin pushed a commit to enjoy-binbin/redis that referenced this pull request Jul 31, 2023
si is initialized by streamIteratorStart(), we should call
streamIteratorStop() on it when done.

regression introduced in redis#9127 (redis 7.0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-notes indication that this issue needs to be mentioned in the release notes state:to-be-merged The PR should be merged soon, even if not yet ready, this is used so that it won't be forgotten

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] memory leak in streamGetEdgeID

3 participants