Skip to content
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

Infinite loop causing OOM when walking stack in minidump-processor #428

Closed
5225225 opened this issue Feb 3, 2022 · 3 comments · Fixed by #430
Closed

Infinite loop causing OOM when walking stack in minidump-processor #428

5225225 opened this issue Feb 3, 2022 · 3 comments · Fixed by #430

Comments

@5225225
Copy link
Contributor

5225225 commented Feb 3, 2022

On commit ebbee33, running cargo fuzz run process oom (attached below in a .zip file) will infinitely loop in the following while statement, pushing more stuff to the frames vec until you eventually OOM.

https://github.com/luser/rust-minidump/blob/a32419975ba6f190081a148f04a41fc42aa4eb26/minidump-processor/src/stackwalker/mod.rs#L174-L193

oom.zip

Bailing out if you ever see a frame twice sounds smart.

Are instruction pointers always monotonically increasing/decreasing? If so, we can do this perfectly accurately in O(1) per frame by just checking that the instruction pointer is always increasing or decreasing by at least 1 byte per frame.

@Gankra
Copy link
Collaborator

Gankra commented Feb 3, 2022

We already enforce forward-progress but it appears I forgot to port the Arm fix in #309 to Arm64 and OldArm64

@Gankra
Copy link
Collaborator

Gankra commented Feb 3, 2022

(Arm leaf functions can genuinely not use the stack at all because the link register creates a buffer of one call where you don't even need to push a return pointer, so the unwinder needs to allow forward progress to not occur for the top of the stack, but after that it needs to be strict again)

@Gankra
Copy link
Collaborator

Gankra commented Feb 3, 2022

(So very good catch, this a real annoying bug we've seen in the wild!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants