Skip to content

Commit e6a0b59

Browse files
[2.7] bpo-27945: Fixed various segfaults with dict. (GH-1657) (#1681)
Based on patches by Duane Griffin and Tim Mitchell. (cherry picked from commit 753bca3)
1 parent e9f9b04 commit e6a0b59

File tree

4 files changed

+127
-14
lines changed

4 files changed

+127
-14
lines changed

Lib/test/test_dict.py

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -690,6 +690,97 @@ def test_free_after_iterating(self):
690690
test_support.check_free_after_iterating(self, lambda d: iter(d.viewvalues()), dict)
691691
test_support.check_free_after_iterating(self, lambda d: iter(d.viewitems()), dict)
692692

693+
def test_equal_operator_modifying_operand(self):
694+
# test fix for seg fault reported in issue 27945 part 3.
695+
class X(object):
696+
def __del__(self):
697+
dict_b.clear()
698+
699+
def __eq__(self, other):
700+
dict_a.clear()
701+
return True
702+
703+
def __hash__(self):
704+
return 13
705+
706+
dict_a = {X(): 0}
707+
dict_b = {X(): X()}
708+
self.assertTrue(dict_a == dict_b)
709+
710+
def test_fromkeys_operator_modifying_dict_operand(self):
711+
# test fix for seg fault reported in issue 27945 part 4a.
712+
class X(int):
713+
def __hash__(self):
714+
return 13
715+
716+
def __eq__(self, other):
717+
if len(d) > 1:
718+
d.clear()
719+
return False
720+
721+
d = {} # this is required to exist so that d can be constructed!
722+
d = {X(1): 1, X(2): 2}
723+
try:
724+
dict.fromkeys(d) # shouldn't crash
725+
except RuntimeError: # implementation defined
726+
pass
727+
728+
def test_fromkeys_operator_modifying_set_operand(self):
729+
# test fix for seg fault reported in issue 27945 part 4b.
730+
class X(int):
731+
def __hash__(self):
732+
return 13
733+
734+
def __eq__(self, other):
735+
if len(d) > 1:
736+
d.clear()
737+
return False
738+
739+
d = {} # this is required to exist so that d can be constructed!
740+
d = {X(1), X(2)}
741+
try:
742+
dict.fromkeys(d) # shouldn't crash
743+
except RuntimeError: # implementation defined
744+
pass
745+
746+
def test_dictitems_contains_use_after_free(self):
747+
class X(object):
748+
def __eq__(self, other):
749+
d.clear()
750+
return NotImplemented
751+
752+
__hash__ = object.__hash__ # silence Py3k warning
753+
754+
d = {0: set()}
755+
try:
756+
(0, X()) in d.iteritems() # shouldn't crash
757+
except RuntimeError: # implementation defined
758+
pass
759+
760+
def test_init_use_after_free(self):
761+
class X(object):
762+
def __hash__(self):
763+
pair[:] = []
764+
return 13
765+
766+
pair = [X(), 123]
767+
dict([pair])
768+
769+
def test_oob_indexing_dictiter_iternextitem(self):
770+
class X(int):
771+
def __del__(self):
772+
d.clear()
773+
774+
d = {i: X(i) for i in range(8)}
775+
776+
def iter_and_mutate():
777+
for result in d.iteritems():
778+
if result[0] == 2:
779+
d[2] = None # free d[2] --> X(2).__del__ was called
780+
781+
self.assertRaises(RuntimeError, iter_and_mutate)
782+
783+
693784
from test import mapping_tests
694785

695786
class GeneralMappingTests(mapping_tests.BasicTestMappingProtocol):

Misc/ACKS

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -500,6 +500,7 @@ Hans de Graaff
500500
Tim Graham
501501
Nathaniel Gray
502502
Eddy De Greef
503+
Duane Griffin
503504
Grant Griffin
504505
Andrea Griffini
505506
Duncan Grisby

Misc/NEWS

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,10 @@ What's New in Python 2.7.14?
1010
Core and Builtins
1111
-----------------
1212

13+
- bpo-27945: Fixed various segfaults with dict when input collections are
14+
mutated during searching, inserting or comparing. Based on patches by
15+
Duane Griffin and Tim Mitchell.
16+
1317
- bpo-25794: Fixed type.__setattr__() and type.__delattr__() for
1418
non-interned or unicode attribute names. Based on patch by Eryk Sun.
1519

Objects/dictobject.c

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1595,11 +1595,18 @@ PyDict_MergeFromSeq2(PyObject *d, PyObject *seq2, int override)
15951595
/* Update/merge with this (key, value) pair. */
15961596
key = PySequence_Fast_GET_ITEM(fast, 0);
15971597
value = PySequence_Fast_GET_ITEM(fast, 1);
1598+
Py_INCREF(key);
1599+
Py_INCREF(value);
15981600
if (override || PyDict_GetItem(d, key) == NULL) {
15991601
int status = PyDict_SetItem(d, key, value);
1600-
if (status < 0)
1602+
if (status < 0) {
1603+
Py_DECREF(key);
1604+
Py_DECREF(value);
16011605
goto Fail;
1606+
}
16021607
}
1608+
Py_DECREF(key);
1609+
Py_DECREF(value);
16031610
Py_DECREF(fast);
16041611
Py_DECREF(item);
16051612
}
@@ -1938,12 +1945,13 @@ dict_equal(PyDictObject *a, PyDictObject *b)
19381945
/* ditto for key */
19391946
Py_INCREF(key);
19401947
bval = PyDict_GetItem((PyObject *)b, key);
1941-
Py_DECREF(key);
19421948
if (bval == NULL) {
1949+
Py_DECREF(key);
19431950
Py_DECREF(aval);
19441951
return 0;
19451952
}
19461953
cmp = PyObject_RichCompareBool(aval, bval, Py_EQ);
1954+
Py_DECREF(key);
19471955
Py_DECREF(aval);
19481956
if (cmp <= 0) /* error or not equal */
19491957
return cmp;
@@ -2743,7 +2751,7 @@ PyTypeObject PyDictIterValue_Type = {
27432751

27442752
static PyObject *dictiter_iternextitem(dictiterobject *di)
27452753
{
2746-
PyObject *key, *value, *result = di->di_result;
2754+
PyObject *key, *value, *result;
27472755
register Py_ssize_t i, mask;
27482756
register PyDictEntry *ep;
27492757
PyDictObject *d = di->di_dict;
@@ -2770,22 +2778,27 @@ static PyObject *dictiter_iternextitem(dictiterobject *di)
27702778
if (i > mask)
27712779
goto fail;
27722780

2773-
if (result->ob_refcnt == 1) {
2781+
di->len--;
2782+
key = ep[i].me_key;
2783+
value = ep[i].me_value;
2784+
Py_INCREF(key);
2785+
Py_INCREF(value);
2786+
result = di->di_result;
2787+
if (Py_REFCNT(result) == 1) {
2788+
PyObject *oldkey = PyTuple_GET_ITEM(result, 0);
2789+
PyObject *oldvalue = PyTuple_GET_ITEM(result, 1);
2790+
PyTuple_SET_ITEM(result, 0, key); /* steals reference */
2791+
PyTuple_SET_ITEM(result, 1, value); /* steals reference */
27742792
Py_INCREF(result);
2775-
Py_DECREF(PyTuple_GET_ITEM(result, 0));
2776-
Py_DECREF(PyTuple_GET_ITEM(result, 1));
2793+
Py_DECREF(oldkey);
2794+
Py_DECREF(oldvalue);
27772795
} else {
27782796
result = PyTuple_New(2);
27792797
if (result == NULL)
27802798
return NULL;
2799+
PyTuple_SET_ITEM(result, 0, key); /* steals reference */
2800+
PyTuple_SET_ITEM(result, 1, value); /* steals reference */
27812801
}
2782-
di->len--;
2783-
key = ep[i].me_key;
2784-
value = ep[i].me_value;
2785-
Py_INCREF(key);
2786-
Py_INCREF(value);
2787-
PyTuple_SET_ITEM(result, 0, key);
2788-
PyTuple_SET_ITEM(result, 1, value);
27892802
return result;
27902803

27912804
fail:
@@ -3186,6 +3199,7 @@ dictitems_iter(dictviewobject *dv)
31863199
static int
31873200
dictitems_contains(dictviewobject *dv, PyObject *obj)
31883201
{
3202+
int result;
31893203
PyObject *key, *value, *found;
31903204
if (dv->dv_dict == NULL)
31913205
return 0;
@@ -3199,7 +3213,10 @@ dictitems_contains(dictviewobject *dv, PyObject *obj)
31993213
return -1;
32003214
return 0;
32013215
}
3202-
return PyObject_RichCompareBool(value, found, Py_EQ);
3216+
Py_INCREF(found);
3217+
result = PyObject_RichCompareBool(value, found, Py_EQ);
3218+
Py_DECREF(found);
3219+
return result;
32033220
}
32043221

32053222
static PySequenceMethods dictitems_as_sequence = {

0 commit comments

Comments
 (0)