8000 [3.4] [3.5] bpo-27945: Fixed various segfaults with dict. (GH-1657) (… · stackless-dev/stackless@f734479 · GitHub
[go: up one dir, main page]

Skip to content
This repository was archived by the owner on Feb 13, 2025. It is now read-only.

Commit f734479

Browse files
serhiy-storchakalarryhastings
authored andcommitted
[3.4] [3.5] bpo-27945: Fixed various segfaults with dict. (pythonGH-1657) (pythonGH-1678) (python#2248)
Based on patches by Duane Griffin and Tim Mitchell. (cherry picked from commit 753bca3). (cherry picked from commit 2f7f533)
1 parent fe82c46 commit f734479

File tree

4 files changed

+143
-30
lines changed

4 files changed

+143
-30
lines changed

Lib/test/test_dict.py

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -952,6 +952,92 @@ def __eq__(self, o):
952952
d = {X(): 0, 1: 1}
953953
self.assertRaises(RuntimeError, d.update, other)
954954

955+
def test_equal_operator_modifying_operand(self):
956+
# test fix for seg fault reported in issue 27945 part 3.
957+
class X():
958+
def __del__(self):
959+
dict_b.clear()
960+
961+
def __eq__(self, other):
962+
dict_a.clear()
963+
return True
964+
965+
def __hash__(self):
966+
return 13
967+
968+
dict_a = {X(): 0}
969+
dict_b = {X(): X()}
970+
self.assertTrue(dict_a == dict_b)
971+
972+
def test_fromkeys_operator_modifying_dict_operand(self):
973+
# test fix for seg fault reported in issue 27945 part 4a.
974+
class X(int):
975+
def __hash__(self):
976+
return 13
977+
978+
def __eq__(self, other):
979+
if len(d) > 1:
980+
d.clear()
981+
return False
982+
983+
d = {} # this is required to exist so that d can be constructed!
984+
d = {X(1): 1, X(2): 2}
985+
try:
986+
dict.fromkeys(d) # shouldn't crash
987+
except RuntimeError: # implementation defined
988+
pass
989+
990+
def test_fromkeys_operator_modifying_set_operand(self):
991+
# test fix for seg fault reported in issue 27945 part 4b.
992+
class X(int):
993+
def __hash__(self):
994+
return 13
995+
996+
def __eq__(self, other):
997+
if len(d) > 1:
998+
d.clear()
999+
return False
1000+
1001+
d = {} # this is required to exist so that d can be constructed!
1002+
d = {X(1), X(2)}
1003+
try:
1004+
dict.fromkeys(d) # shouldn't crash
1005+
except RuntimeError: # implementation defined
1006+
pass
1007+
1008+
def test_dictitems_contains_use_after_free(self):
1009+
class X:
1010+
def __eq__(self, other):
1011+
d.clear()
1012+
return NotImplemented
1013+
1014+
d = {0: set()}
1015+
(0, X()) in d.items()
1016+
1017+
def test_init_use_after_free(self):
1018+
class X:
1019+
def __hash__(self):
1020+
pair[:] = []
1021+
return 13
1022+
1023+
pair = [X(), 123]
1024+
dict([pair])
1025+
1026+
def test_oob_indexing_dictiter_iternextitem(self):
1027+
class X(int):
1028+
def __del__(self):
1029+
d.clear()
1030+
1031+
d = {i: X(i) for i in range(8)}
1032+
1033+
def iter_and_mutate():
1034+
for result in d.items():
1035+
if result[0] == 2:
1036+
d[2] = None # free d[2] --> X(2).__del__ was called
1037+
1038+
self.assertRaises(RuntimeError, iter_and_mutate)
1039+
1040+
9551041
from test import mapping_tests
9561042

9571043
class GeneralMappingTests(mapping_tests.BasicTestMappingProtocol):

Misc/ACKS

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -506,6 +506,7 @@ Hans de Graaff
506506
Tim Graham
507507
Nathaniel Gray
508508
Eddy De Greef
509+
Duane Griffin
509510
Grant Griffin
510511
Andrea Griffini
511512
Duncan Grisby

Misc/NEWS

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
+++++++++++
1+
+++++++++++
22
Python News
33
+++++++++++
44

@@ -10,6 +10,10 @@ Release date: XXXX-XX-XX
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
Documentation
1418
-------------
1519

Objects/dictobject.c

Lines changed: 51 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -805,56 +805,61 @@ insertdict(PyDictObject *mp, PyObject *key, Py_hash_t hash, PyObject *value)
805805
PyDictKeyEntry *ep;
806806
assert(key != dummy);
807807

808+
Py_INCREF(key);
809+
Py_INCREF(value);
808810
if (mp->ma_values != NULL && !PyUnicode_CheckExact(key)) {
809811
if (insertion_resize(mp) < 0)
810-
return -1;
812+
goto Fail;
811813
}
812814

813815
ep = mp->ma_keys->dk_lookup(mp, key, hash, &value_addr);
814-
if (ep == NULL) {
815-
return -1;
816-
}
816+
if (ep == NULL)
817+
goto Fail;
818+
817819
assert(PyUnicode_CheckExact(key) || mp->ma_keys->dk_lookup == lookdict);
818-
Py_INCREF(value);
819820
MAINTAIN_TRACKING(mp, key, value);
820821
old_value = *value_addr;
821822
if (old_value != NULL) {
822823
assert(ep->me_key != NULL && ep->me_key != dummy);
823824
*value_addr = value;
824825
Py_DECREF(old_value); /* which **CAN** re-enter (see issue #22653) */
826+
Py_DECREF(key);
825827
}
826828
else {
827829
if (ep->me_key == NULL) {
828-
Py_INCREF(key);
829830
if (mp->ma_keys->dk_usable <= 0) {
830831
/* Need to resize. */
831-
if (insertion_resize(mp) < 0) {
832-
Py_DECREF(key);
833-
Py_DECREF(value);
834-
return -1;
835-
}
832+
if (insertion_resize(mp) < 0)
833+
goto Fail;
836834
ep = find_empty_slot(mp, key, hash, &value_addr);
837835
}
836+
mp->ma_used++;
837+
*value_addr = value;
838838
mp->ma_keys->dk_usable--;
839839
assert(mp->ma_keys->dk_usable >= 0);
840840
ep->me_key = key;
841841
ep->me_hash = hash;
842+
assert(ep->me_key != NULL && ep->me_key != dummy);
842843
}
843844
else {
845+
mp->ma_used++;
846+
*value_addr = value;
844847
if (ep->me_key == dummy) {
845-
Py_INCREF(key);
846848
ep->me_key = key;
847849
ep->me_hash = hash;
848850
Py_DECREF(dummy);
849851
} else {
850852
assert(_PyDict_HasSplitTable(mp));
853+
Py_DECREF(key);
851854
}
852855
}
853-
mp->ma_used++;
854-
*value_addr = value;
855-
assert(ep->me_key != NULL && ep->me_key != dummy);
856856
}
857857
return 0;
858+
859+
Fail:
860+
Py_DECREF(value);
861+
Py_DECREF(key);
862+
return -1;
858863
}
859864

860865
/*
@@ -1911,11 +1916,18 @@ PyDict_MergeFromSeq2(PyObject *d, PyObject *seq2, int override)
19111916
/* Update/merge with this (key, value) pair. */
19121917
key = PySequence_Fast_GET_ITEM(fast, 0);
19131918
value = PySequence_Fast_GET_ITEM(fast, 1);
1919+
Py_INCREF(key);
1920+
Py_INCREF(value);
19141921
if (override || PyDict_GetItem(d, key) == NULL) {
19151922
int status = PyDict_SetItem(d, key, value);
1916-
if (status < 0)
1923+
if (status < 0) {
1924+
Py_DECREF(key);
1925+
Py_DECREF(value);
19171926
goto Fail;
1927+
}
19181928
}
1929+
Py_DECREF(key);
1930+
Py_DECREF(value);
19191931
Py_DECREF(fast);
19201932
Py_DECREF(item);
19211933
}
@@ -2174,14 +2186,15 @@ dict_equal(PyDictObject *a, PyDictObject *b)
21742186
bval = NULL;
21752187
else
21762188
bval = *vaddr;
2177-
Py_DECREF(key);
21782189
if (bval == NULL) {
2190+
Py_DECREF(key);
21792191
Py_DECREF(aval);
21802192
if (PyErr_Occurred())
21812193
return -1;
21822194
return 0;
21832195
}
21842196
cmp = PyObject_RichCompareBool(aval, bval, Py_EQ);
2197+
Py_DECREF(key);
21852198
Py_DECREF(aval);
21862199
if (cmp <= 0) /* error or not equal */
21872200
return cmp;
@@ -3046,7 +3059,7 @@ PyTypeObject PyDictIterValue_Type = {
30463059

30473060
static PyObject *dictiter_iternextitem(dictiterobject *di)
30483061
{
3049-
PyObject *key, *value, *result = di->di_result;
3062+
PyObject *key, *value, *result;
30503063
Py_ssize_t i, mask, offset;
30513064
PyDictObject *d = di->di_dict;
30523065
PyObject **value_ptr;
@@ -3082,22 +3095,27 @@ static PyObject *dictiter_iternextitem(dictiterobject *di)
30823095
if (i > mask)
30833096
goto fail;
30843097

3085-
if (result->ob_refcnt == 1) {
3098+
di->len--;
3099+
key = d->ma_keys->dk_entries[i].me_key;
3100+
value = *value_ptr;
3101+
Py_INCREF(key);
3102+
Py_INCREF(value);
3103+
result = di->di_result;
3104+
if (Py_REFCNT(result) == 1) {
3105+
PyObject *oldkey = PyTuple_GET_ITEM(result, 0);
3106+
PyObject *oldvalue = PyTuple_GET_ITEM(result, 1);
3107+
PyTuple_SET_ITEM(result, 0, key); /* steals reference */
3108+
PyTuple_SET_ITEM(result, 1, value); /* steals reference */
30863109
Py_INCREF(result);
3087-
Py_DECREF(PyTuple_GET_ITEM(result, 0));
3088-
Py_DECREF(PyTuple_GET_ITEM(result, 1));
3110+
Py_DECREF(oldkey);
3111+
Py_DECREF(oldvalue);
30893112
} else {
30903113
result = PyTuple_New(2);
30913114
if (result == NULL)
30923115
return NULL;
3116+
PyTuple_SET_ITEM(result, 0, key); /* steals reference */
3117+
PyTuple_SET_ITEM(result, 1, value); /* steals reference */
30933118
}
3094-
di->len--;
3095-
key = d->ma_keys->dk_entries[i].me_key;
3096-
value = *value_ptr;
3097-
Py_INCREF(key);
3098-
Py_INCREF(value);
3099-
PyTuple_SET_ITEM(result, 0, key);
3100-
PyTuple_SET_ITEM(result, 1, value);
31013119
return result;
31023120

31033121
fail:
@@ -3596,6 +3614,7 @@ dictitems_iter(dictviewobject *dv)
35963614
static int
35973615
dictitems_contains(dictviewobject *dv, PyObject *obj)
35983616
{
3617+
int result;
35993618
PyObject *key, *value, *found;
36003619
if (dv->dv_dict == NULL)
36013620
return 0;
@@ -3609,7 +3628,10 @@ dictitems_contains(dictviewobject *dv, PyObject *obj)
36093628
return -1;
36103629
return 0;
36113630
}
3612-
return PyObject_RichCompareBool(value, found, Py_EQ);
3631+
Py_INCREF(found);
3632+
result = PyObject_RichCompareBool(value, found, Py_EQ);
3633+
Py_DECREF(found);
3634+
return result;
36133635
}
36143636

36153637
static PySequenceMethods dictitems_as_sequence = {

0 commit comments

Comments
 (0)
0