1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

QXpmHandler: fix re-entrancy bug in xpm_color_name

Browse Source 
The xpm_color_name() function returned a pointer to a function-static
buffer. This is infamously non-reentrant, and an actual problem,
because we explicitly allow QImage operations (incl. saving to an
.xpm) from non-GUI-threads.

Fix by using the CSS pattern (Caller-Supplied Storage; also used in
the QAnyStringView(char32_t) and QAnyStringView(QStringBuilder) ctors)
to force the caller to allocate storage in its own stack frame. As a
consequence, we re-gain re-entrancy, but the returned pointer is now
only valid until the end of the full-expression, which necessitated
simplifying one caller (sorry!).

To see why said simplification is valid, observe that xpm_color_name()
writes a (now-explicit) NUL into returnable[cpp] and the old code read
max(cpp, 4) characters from xpm_color_name()'s result.

NB: cpp can be 5, even though the code comments say otherwise! :(

[ChangeLog][QtGui][QImage] Fixed a race condition when concurrently
writing .xpm files.

Change-Id: I36d7173d53839a52f5cdf58324474c1b32c71f33
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
(cherry picked from commit 73fabadcee71af858388fb245fccf4e96d4ead4e)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
This commit is contained in:
Marc Mutz 2021-08-04 10:17:11 +02:00 committed by Qt Cherry-pick Bot
parent aa23eb493c
commit 36760514a9
1 changed files with 13 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
src/gui/image/qxpmhandler.cpp
View File

@ -52,6 +52,7 @@
#include <private/qcolor_p.h> #include <private/qcolor_p.h>
#include <algorithm> #include <algorithm>
#include <array>
QT_BEGIN_NAMESPACE QT_BEGIN_NAMESPACE
@ -1063,15 +1064,23 @@ bool qt_read_xpm_image_or_array(QIODevice *device, const char * const * source,
return read_xpm_body(device, source, index, state, cpp, ncols, w, h, image); return read_xpm_body(device, source, index, state, cpp, ncols, w, h, image);
} }
static const char* xpm_color_name(int cpp, int index) namespace {
template <size_t N>
struct CharBuffer : std::array<char, N>
{
CharBuffer() {} // avoid value-initializing the whole array
};
}
static const char* xpm_color_name(int cpp, int index, CharBuffer<5> && returnable = {})
{ {
static char returnable[5];
static const char code[] = ".#abcdefghijklmnopqrstuvwxyzABCD" static const char code[] = ".#abcdefghijklmnopqrstuvwxyzABCD"
"EFGHIJKLMNOPQRSTUVWXYZ0123456789"; "EFGHIJKLMNOPQRSTUVWXYZ0123456789";
// cpp is limited to 4 and index is limited to 64^cpp // cpp is limited to 4 and index is limited to 64^cpp
if (cpp > 1) { if (cpp > 1) {
if (cpp > 2) { if (cpp > 2) {
if (cpp > 3) { if (cpp > 3) {
returnable[4] = '\0';
returnable[3] = code[index % 64]; returnable[3] = code[index % 64];
index /= 64; index /= 64;
} else } else
@ -1091,7 +1100,7 @@ static const char* xpm_color_name(int cpp, int index)
returnable[1] = '\0'; returnable[1] = '\0';
returnable[0] = code[index]; returnable[0] = code[index];
return returnable; return returnable.data();
} }
@ -1157,16 +1166,7 @@ static bool write_xpm_image(const QImage &sourceImage, QIODevice *device, const
const QRgb *yp = reinterpret_cast<const QRgb *>(image.constScanLine(y)); const QRgb *yp = reinterpret_cast<const QRgb *>(image.constScanLine(y));
for(x=0; x<w; x++) { for(x=0; x<w; x++) {
int color = (int)(*(yp + x)); int color = (int)(*(yp + x));
const QByteArray chars(xpm_color_name(cpp, colorMap[color])); line.append(xpm_color_name(cpp, colorMap[color]));
line.append(chars[0]);
if (cpp > 1) {
line.append(chars[1]);
if (cpp > 2) {
line.append(chars[2]);
if (cpp > 3)
line.append(chars[3]);
}
}
} }
s << ',' << Qt::endl << '\"' << line << '\"'; s << ',' << Qt::endl << '\"' << line << '\"';
} }