Crash: Use after free of std::function through CodepointWidthDetector #2375
Assignees
Labels
Area-Server
Down in the muck of API call servicing, interprocess communication, eventing, etc.
Issue-Bug
It either shouldn't be doing this or needs an investigation.
Needs-Tag-Fix
Doesn't match tag requirements
Product-Conhost
For issues in the Console codebase
Product-Terminal
The new Windows Terminal.
Resolution-Fix-Committed
Fix is checked in, but it might be 3-4 weeks until a release.
Severity-Crash
Crashes are real bad news.
Comments
ghost
added
Needs-Triage
|
Did someone set a callback pointing to a GUID? |
|
This is more likely a use-after-free or an std::function parked on an invalid address than an actual callback pointing to a GUID 😁 |
|
Hmm, after ~1 week without any crash, I just hit a second in the space of a few minutes. Same symptom (std::invoke callback gone awry), different stack. |
DHowett-MSFT
changed the title
DHowett-MSFT
added
Area-Server
|
This looks like a UAF of probably a dessicated husk of a All of the CABs @0xabu provided lead to the same sad end. Thanks! |
|
Oh jeez, it's because we have a singleton codepoint width detector and we bind its width detector fallback to an instance of a single terminal control's renderer. This also means that the answer for glyph widths in one tab will change based on the font in the most recently created tab. |
This was referenced Sep 13, 2019
Closed
|
Assigning Dustin because we talked about this briefly in the hallway and I want to remember to not pick it up out from under what his idea is here. |
DHowett-MSFT
pushed a commit
that referenced
this issue
Sep 27, 2019
From Egmont Koblinger: > In terminal emulation, apps have to be able to print something and keep track of the cursor, whereas they by design have no idea of the font being used. In many terminals the font can also be changed runtime and it's absolutely not feasible to then rearrange the cells. In some other cases there is no font at all (e.g. the libvterm headless terminal emulation library, or a detached screen/tmux), or there are multiple fonts at once (a screen/tmux attached from multiple graphical emulators). > The only way to do that is via some external agreement on the number of cells, which is typically the Unicode EastAsianWidth, often accessed via wcwidth(). It's not perfect (changes through Unicode versions, has ambiguous characters, etc.) but is still the best we have. > glibc's wcwidth() reports 1 for ambiguous width characters, so the de facto standard is that in terminals they are narrow. > If the glyph is wider then the terminal has to figure out what to do. It could crop it (newer versions of Konsole, as far as I know), overflow to the right (VTE), shrink it (Kitty I believe does this), etc. See Also: https://bugzilla.gnome.org/show_bug.cgi?id=767529 https://gitlab.freedesktop.org/terminal-wg/specifications/issues/9 https://www.unicode.org/reports/tr11/tr11-34.html Salient point from proposed update to Unicode Standard Annex #11: > Note: The East_Asian_Width property is not intended for use by modern terminal emulators without appropriate tailoring on a case-by-case basis. Fixes #2066 Fixes #2375
5 tasks
This was referenced Sep 27, 2019
DHowett-MSFT
pushed a commit
that referenced
this issue
Oct 15, 2019
From Egmont Koblinger: > In terminal emulation, apps have to be able to print something and keep track of the cursor, whereas they by design have no idea of the font being used. In many terminals the font can also be changed runtime and it's absolutely not feasible to then rearrange the cells. In some other cases there is no font at all (e.g. the libvterm headless terminal emulation library, or a detached screen/tmux), or there are multiple fonts at once (a screen/tmux attached from multiple graphical emulators). > The only way to do that is via some external agreement on the number of cells, which is typically the Unicode EastAsianWidth, often accessed via wcwidth(). It's not perfect (changes through Unicode versions, has ambiguous characters, etc.) but is still the best we have. > glibc's wcwidth() reports 1 for ambiguous width characters, so the de facto standard is that in terminals they are narrow. > If the glyph is wider then the terminal has to figure out what to do. It could crop it (newer versions of Konsole, as far as I know), overflow to the right (VTE), shrink it (Kitty I believe does this), etc. See Also: https://bugzilla.gnome.org/show_bug.cgi?id=767529 https://gitlab.freedesktop.org/terminal-wg/specifications/issues/9 https://www.unicode.org/reports/tr11/tr11-34.html Salient point from proposed update to Unicode Standard Annex 11: > Note: The East_Asian_Width property is not intended for use by modern terminal emulators without appropriate tailoring on a case-by-case basis. Fixes #2066 Fixes #2375 Related to #900
ghost
added
Needs-Tag-Fix
|
🎉This issue was addressed in #2928, which has now been successfully released as Handy links: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Environment
Steps to reproduce
I'm honestly not sure what I was doing. This might be #2251?
Expected behavior
The terminal shouldn't crash.
Actual behavior
I'm happy to share the crash dump out of band.
The text was updated successfully, but these errors were encountered: