diff --git a/Lib/test/crashers/losing_mro_ref.py b/Lib/test/crashers/losing_mro_ref.py deleted file mode 100644 index b3bcd32c58c..00000000000 --- a/Lib/test/crashers/losing_mro_ref.py +++ /dev/null @@ -1,35 +0,0 @@ -""" -There is a way to put keys of any type in a type's dictionary. -I think this allows various kinds of crashes, but so far I have only -found a convoluted attack of _PyType_Lookup(), which uses the mro of the -type without holding a strong reference to it. Probably works with -super.__getattribute__() too, which uses the same kind of code. -""" - -class MyKey(object): - def __hash__(self): - return hash('mykey') - - def __eq__(self, other): - # the following line decrefs the previous X.__mro__ - X.__bases__ = (Base2,) - # trash all tuples of length 3, to make sure that the items of - # the previous X.__mro__ are really garbage - z = [] - for i in range(1000): - z.append((i, None, None)) - return 0 - - -class Base(object): - mykey = 'from Base' - -class Base2(object): - mykey = 'from Base2' - -# you can't add a non-string key to X.__dict__, but it can be -# there from the beginning :-) -X = type('X', (Base,), {MyKey(): 5}) - -print(X.mykey) -# I get a segfault, or a slightly wrong assertion error in a debug build. diff --git a/Lib/test/test_descr.py b/Lib/test/test_descr.py index 45fd05dd4ba..f405fbbb572 100644 --- a/Lib/test/test_descr.py +++ b/Lib/test/test_descr.py @@ -4582,10 +4582,38 @@ class PTypesLongInitTest(unittest.TestCase): type.mro(tuple) +class MiscTests(unittest.TestCase): + def test_type_lookup_mro_reference(self): + # Issue #14199: _PyType_Lookup() has to keep a strong reference to + # the type MRO because it may be modified during the lookup, if + # __bases__ is set during the lookup for example. + class MyKey(object): + def __hash__(self): + return hash('mykey') + + def __eq__(self, other): + X.__bases__ = (Base2,) + + class Base(object): + mykey = 'from Base' + mykey2 = 'from Base' + + class Base2(object): + mykey = 'from Base2' + mykey2 = 'from Base2' + + X = type('X', (Base,), {MyKey(): 5}) + # mykey is read from Base + self.assertEqual(X.mykey, 'from Base') + # mykey2 is read from Base2 because MyKey.__eq__ has set __bases__ + self.assertEqual(X.mykey2, 'from Base2') + + def test_main(): # Run all local test cases, with PTypesLongInitTest first. support.run_unittest(PTypesLongInitTest, OperatorsTest, - ClassPropertiesAndMethods, DictProxyTests) + ClassPropertiesAndMethods, DictProxyTests, + MiscTests) if __name__ == "__main__": test_main() diff --git a/Objects/typeobject.c b/Objects/typeobject.c index dae7ff8d9a7..44873226093 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -2450,6 +2450,9 @@ _PyType_Lookup(PyTypeObject *type, PyObject *name) return NULL; res = NULL; + /* keep a strong reference to mro because type->tp_mro can be replaced + during PyDict_GetItem(dict, name) */ + Py_INCREF(mro); assert(PyTuple_Check(mro)); n = PyTuple_GET_SIZE(mro); for (i = 0; i < n; i++) { @@ -2461,6 +2464,7 @@ _PyType_Lookup(PyTypeObject *type, PyObject *name) if (res != NULL) break; } + Py_DECREF(mro); if (MCACHE_CACHEABLE_NAME(name) && assign_version_tag(type)) { h = MCACHE_HASH_METHOD(type, name); @@ -6281,6 +6285,9 @@ super_getattro(PyObject *self, PyObject *name) } i++; res = NULL; + /* keep a strong reference to mro because starttype->tp_mro can be + replaced during PyDict_GetItem(dict, name) */ + Py_INCREF(mro); for (; i < n; i++) { tmp = PyTuple_GET_ITEM(mro, i); if (PyType_Check(tmp)) @@ -6305,9 +6312,11 @@ super_getattro(PyObject *self, PyObject *name) Py_DECREF(res); res = tmp; } + Py_DECREF(mro); return res; } } + Py_DECREF(mro); } return PyObject_GenericGetAttr(self, name); }