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.

Sign up for GitHub

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

Jump to bottom

Fix toxav use after free caused by premature MSI destruction #353

Merged
merged 1 commit into from
Dec 20, 2016
Merged

Fix toxav use after free caused by premature MSI destruction #353

merged 1 commit into from
Dec 20, 2016

Conversation

mannol
Copy link

@mannol mannol commented Dec 20, 2016 •
edited by iphydf
Loading

This change is Reviewable

@mannol mannol requested a review from iphydf December 20, 2016 21:35
@mannol mannol assigned mannol and iphydf and unassigned mannol Dec 20, 2016
@iphydf iphydf added this to the v0.1.3 milestone Dec 20, 2016
@iphydf
Copy link
Member

iphydf commented Dec 20, 2016

Please change the commit message to explain what this is doing. You can add "Fixes #278" in the second paragraph of the commit message. Given only this message, I see that it fixes something, but I don't understand in what way it was broken, and whether this fix has any other implications (potential memory leak?), and if not, why not.

iphydf
iphydf requested changes Dec 20, 2016
View reviewed changes
Copy link
Member

@iphydf iphydf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Improve commit message.

@iphydf iphydf modified the milestones: v0.1.3, v0.1.2 Dec 20, 2016
Use after free reported in #278 occurs because toxav_kill()
7122d2e 
calls msi_kill() (toxav.c:180) which frees msi_call instances (msi.c:161)
which are then used when call_remove() (toxav.c:1136) is called.
This fix prevents call_remove() from calling invalid pointer.

Fixes #278
@mannol
Copy link
Author

mannol commented Dec 20, 2016

@iphydf done!

@iphydf iphydf changed the title Fix #278 Use after free reported in #278 occurs because toxav_kill() Dec 20, 2016
@iphydf
Copy link
Member

iphydf commented Dec 20, 2016

:lgtm:

Reviewed 1 of 1 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

@iphydf iphydf changed the title Use after free reported in #278 occurs because toxav_kill() Fix toxav use after free caused by premature MSI destruction Dec 20, 2016
@iphydf iphydf merged commit 7122d2e into TokTok:master Dec 20, 2016
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iphydf iphydf iphydf approved these changes

Assignees

@iphydf iphydf

Labels
None yet
Projects
None yet
Milestone
v0.1.2
Development

Successfully merging this pull request may close these issues.
2 participants
@mannol @iphydf