-
Notifications
You must be signed in to change notification settings - Fork 368
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
Harden windows_get_shell
#5714
Harden windows_get_shell
#5714
Conversation
related to #5644 (comment) |
master_changes.md
Outdated
@@ -105,6 +105,7 @@ users) | |||
## Internal | |||
|
|||
## Internal: Windows | |||
* Fix sporadic crash in shell detection (seen in native containers) [#5714 @dra27] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note for dev: Missing api changelog
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
In Windows native containers, the Filename.chop_suffix call in windows_get_shell was occasionally failing. OpamStubs.getProcessAncestry can return empty strings. windows_get_shell now explictly handles the empty string case and .exe is now _optionally_ stripped.
The code for reading new processes would fail to NULL-terminate the list if new entries were read and the target was not found.
The current pointer would end up pointing to a freed block if realloc had to move the process list.
I pushed two commits fixing errors in the C stub for determining the shell, one of which was heinous (the pointer into the array would end up pointing at the old array following |
Thanks! |
In Windows native containers, the
Filename.chop_suffix
call inwindows_get_shell
was occasionally failing.OpamStubs.getProcessAncestry
can return empty strings.windows_get_shell
now explictly handles the empty string case and.exe
is now optionally stripped.