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

[PLAT-6316] Fix SIGSEGV in bsg_ksmachgetThreadQueueName #1065

Merged
merged 1 commit into from
Apr 9, 2021

Conversation

nickdowell
Copy link
Contributor

@nickdowell nickdowell commented Apr 7, 2021

Goal

Fixes a crash that can occur in bsg_ksmachgetThreadQueueName

Changeset

The thread_identifier_info.dispatch_qaddr can sometimes contain an invalid address, which will cause a crash if dereferenced. This is something that has been noted in the Crashlytics source code.

bsg_ksmachcopyMem() is now used to read the contents of the memory without causing a crash if invalid.

Testing

I was not able to reproduce the crash, but manual testing and the E2E tests confirm that the function continues to return the expected queue name.

@nickdowell nickdowell changed the title Fix SIGSEGV in bsg_ksmachgetThreadQueueName [PLAT-6316] Fix SIGSEGV in bsg_ksmachgetThreadQueueName Apr 7, 2021
@nickdowell nickdowell marked this pull request as ready for review April 7, 2021 12:27
@github-actions
Copy link

github-actions bot commented Apr 7, 2021

Infer: No issues found 🎉

OCLint: No issues found 🎉

Bugsnag.framework binary size did not change - 1,102,960 bytes

Generated by 🚫 Danger

Copy link
Contributor

@kattrali kattrali left a comment

Choose a reason for hiding this comment

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

I don't entirely see how this would happen, at least based on the available xnu sources, where the addr is non-zero but also invalid, but a little extra safety is always welcome.

@sethfri
Copy link

sethfri commented Apr 9, 2021

The timing of this is wonderful. I just noticed we've seen 1500 instances of this crash in production and was about to open an issue 😅

@nickdowell nickdowell merged commit f3f37db into next Apr 9, 2021
@nickdowell nickdowell deleted the nickdowell/fix-thread-queue-name-crash branch April 9, 2021 09:42
@nickdowell nickdowell mentioned this pull request Apr 14, 2021
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 this pull request may close these issues.

3 participants