QV4MM: Fix crash caused by MarkStack overflow
MemoryManager::collectFromJSStack did push to the mark stack without checking if there is actually still space available. To fix this, we now drain the stack once we hit the limit. The test case is a slightly modified version compared to the reported one, removing one loop. This was required as our regular expression does not throw an exception when there are too many capture groups. However, to trigger the bug, looping was not actually necessary. Change-Id: I4d00865f25a989c380f4f5b221f4068c80b71d2b Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
This commit is contained in:
parent
09c4ec3202
commit
e72b032cc1
|
@ -1263,8 +1263,11 @@ QUrl ExecutionEngine::resolvedUrl(const QString &file)
|
|||
void ExecutionEngine::markObjects(MarkStack *markStack)
|
||||
{
|
||||
for (int i = 0; i < NClasses; ++i)
|
||||
if (classes[i])
|
||||
if (classes[i]) {
|
||||
classes[i]->mark(markStack);
|
||||
if (markStack->top >= markStack->limit)
|
||||
markStack->drain();
|
||||
}
|
||||
markStack->drain();
|
||||
|
||||
identifierTable->markObjects(markStack);
|
||||
|
|
|
@ -555,6 +555,8 @@ struct ValueArray {
|
|||
} else {
|
||||
while (v < end) {
|
||||
v->mark(markStack);
|
||||
if (markStack->top >= markStack->limit)
|
||||
markStack->drain();
|
||||
++v;
|
||||
}
|
||||
}
|
||||
|
|
|
@ -1219,6 +1219,8 @@ void MemoryManager::collectFromJSStack(MarkStack *markStack) const
|
|||
Q_ASSERT(m->inUse());
|
||||
// Skip pointers to already freed objects, they are bogus as well
|
||||
m->mark(markStack);
|
||||
if (markStack->top >= markStack->limit)
|
||||
markStack->drain();
|
||||
}
|
||||
++v;
|
||||
}
|
||||
|
|
|
@ -381,6 +381,8 @@ private slots:
|
|||
void semicolonAfterProperty();
|
||||
void hugeStack();
|
||||
|
||||
void gcCrashRegressionTest();
|
||||
|
||||
private:
|
||||
// static void propertyVarWeakRefCallback(v8::Persistent<v8::Value> object, void* parameter);
|
||||
static void verifyContextLifetime(QQmlContextData *ctxt);
|
||||
|
@ -9211,6 +9213,53 @@ void tst_qqmlecmascript::hugeStack()
|
|||
QCOMPARE(qvariant_cast<QJSValue>(huge).property(QLatin1String("length")).toInt(), 33059);
|
||||
}
|
||||
|
||||
void tst_qqmlecmascript::gcCrashRegressionTest()
|
||||
{
|
||||
const QString qmljs = QLibraryInfo::location(QLibraryInfo::BinariesPath) + "/qmljs";
|
||||
if (!QFile::exists(qmljs)) {
|
||||
QSKIP("Tets requires qmljs");
|
||||
}
|
||||
QProcess process;
|
||||
|
||||
QTemporaryFile infile;
|
||||
QVERIFY(infile.open());
|
||||
infile.write(R"js(
|
||||
function i_want_to_break_free() {
|
||||
var n = 400;
|
||||
var m = 10;
|
||||
var regex = new RegExp("(ab)".repeat(n), "g"); // g flag to trigger the vulnerable path
|
||||
var part = "ab".repeat(n); // matches have to be at least size 2 to prevent interning
|
||||
var s = (part + "|").repeat(m);
|
||||
var cnt = 0;
|
||||
var ary = [];
|
||||
s.replace(regex, function() {
|
||||
for (var i = 1; i < arguments.length-2; ++i) {
|
||||
if (typeof arguments[i] !== 'string') {
|
||||
i_am_free = arguments[i];
|
||||
throw "success";
|
||||
}
|
||||
ary[cnt++] = arguments[i]; // root everything to force GC
|
||||
}
|
||||
return "x";
|
||||
});
|
||||
}
|
||||
try { i_want_to_break_free(); } catch (e) {console.log("hi") }
|
||||
console.log(typeof(i_am_free)); // will print "object"
|
||||
)js");
|
||||
infile.close();
|
||||
|
||||
QProcessEnvironment environment = QProcessEnvironment::systemEnvironment();
|
||||
environment.insert("QV4_GC_MAX_STACK_SIZE", "32768");
|
||||
|
||||
process.setProcessEnvironment(environment);
|
||||
process.start(qmljs, QStringList({infile.fileName()}));
|
||||
QVERIFY(process.waitForStarted());
|
||||
const qint64 pid = process.processId();
|
||||
QVERIFY(pid != 0);
|
||||
QVERIFY(process.waitForFinished());
|
||||
QCOMPARE(process.exitCode(), 0);
|
||||
}
|
||||
|
||||
QTEST_MAIN(tst_qqmlecmascript)
|
||||
|
||||
#include "tst_qqmlecmascript.moc"
|
||||
|
|
Loading…
Reference in New Issue