Details
-
Bug
-
Resolution: Fixed
-
P1: Critical
-
6.7.0 RC
-
None
-
-
b45d7cc24 (6.7.0), 7a84c58f5 (dev), c9b5b40d1 (6.7)
Description
I'm sorry I don't bring the minimal example, but please hear me out (and read some code). Because I've updated Telegram Desktop to Qt 6.7.0 and got huge amount of crashes, I found the cause and fixed it - it's a regression.
There was this commit for Qt 6.7.0 QTextEngine: Protect against integer overflow with huge texts - the link highlights the problematic lines being changed.
And it does protect (I guess) against integer overflows, but for a-little-bit-large strings, like 4kb, it gets a different UB due to unsigned integer types wrapping around 0 (while before it was a signed negative value). The code is following:
qsizetype space_charAttributes = sizeof(QCharAttributes) * string.size() / sizeof(void*) + 1; qsizetype space_logClusters = sizeof(unsigned short) * string.size() / sizeof(void*) + 1; available_glyphs = (allocated - space_charAttributes - space_logClusters) * sizeof(void*) / QGlyphLayout::SpaceNeeded;
Everything is qsizetype. Before everything was int. Consider a large string: space_charAttributes becomes large, space_logClusters becomes large and it is possible that:
allocated < space_charAttributes + space_char_logClusters
Then before the commit available_glyphs simply became negative and the following check if (available_glyphs < str.size()) was still correctly passed, while now available_glyphs becomes this huge unsigned value (due to the result wrapping around 0) and you get available_glyphs >= str.size(), so you proceed with this code:
memory = stack_memory; ... void *m = memory + space_charAttributes + space_logClusters; glyphLayout = QGlyphLayout(reinterpret_cast<char *>(m), str.size()); glyphLayout.clear();
Here we crash in the memset inside glyphLayout.clear(), because we try to clear str.size() * something amount of memory (which is more than we've allocated on the stack).
So my point is, that should be done:
qsizetype space_charAttributes = sizeof(QCharAttributes) * string.size() / sizeof(void*) + 1; qsizetype space_logClusters = sizeof(unsigned short) * string.size() / sizeof(void*) + 1; if (allocated > space_charAttributes + space_logClusters) { available_glyphs = (allocated - space_charAttributes - space_logClusters) * sizeof(void*) / QGlyphLayout::SpaceNeeded; } else { available_glyphs = 0; }
This patch fixed the crash for me.
Attachments
For Gerrit Dashboard: QTBUG-123339 | ||||||
---|---|---|---|---|---|---|
# | Subject | Branch | Project | Status | CR | V |
548172,2 | Revert "QTextEngine: Protect against integer overflow with huge texts" | 6.7.0 | qt/qtbase | Status: MERGED | +2 | 0 |
548335,2 | Fix QTextEngine regression with large-ish texts | dev | qt/qtbase | Status: MERGED | +2 | 0 |
548393,2 | Fix QTextEngine regression with large-ish texts | 6.7 | qt/qtbase | Status: MERGED | +2 | 0 |