Skip to content

Commit 765d7d5

Browse files
committed
gh-153981: Fix use after free bug itertools.count()
Obtain a reference to lz->long_cnt, lz->long_step such that we don't hit a use after free
1 parent c89b5ef commit 765d7d5

2 files changed

Lines changed: 62 additions & 16 deletions

File tree

Lib/test/test_free_threading/test_itertools.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
batched,
55
chain,
66
combinations_with_replacement,
7+
count,
78
cycle,
89
permutations,
910
tee,
@@ -142,5 +143,32 @@ def consume():
142143
self.assertEqual(errors, [], msg=f"unexpected errors: {errors}")
143144

144145

146+
class TestCountConcurrent(unittest.TestCase):
147+
@staticmethod
148+
def _spin_next(it, n=2000):
149+
for _ in range(n):
150+
next(it)
151+
152+
@staticmethod
153+
def _spin_repr(it, n=2000):
154+
for _ in range(n):
155+
repr(it)
156+
157+
@threading_helper.reap_threads
158+
def test_repr_racing_next_fast_mode(self):
159+
for _ in range(10):
160+
it = count()
161+
workers = [self._spin_next] * 2 + [self._spin_repr] * 4
162+
threading_helper.run_concurrently(workers, args=(it,))
163+
164+
@threading_helper.reap_threads
165+
def test_repr_racing_next_slow_mode(self):
166+
for _ in range(10):
167+
# Large count to trigger "slow mode"
168+
it = count(10**18, 2)
169+
workers = [self._spin_next] * 2 + [self._spin_repr] * 4
170+
threading_helper.run_concurrently(workers, args=(it,))
171+
172+
145173
if __name__ == "__main__":
146174
unittest.main()

Modules/itertoolsmodule.c

Lines changed: 34 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3675,27 +3675,45 @@ static PyObject *
36753675
count_repr(PyObject *op)
36763676
{
36773677
countobject *lz = countobject_CAST(op);
3678-
if (lz->long_cnt == NULL) {
3678+
PyObject *long_cnt, *long_step, *result;
3679+
3680+
Py_BEGIN_CRITICAL_SECTION(lz);
3681+
long_cnt = Py_XNewRef(lz->long_cnt);
3682+
long_step = Py_XNewRef(lz->long_step);
3683+
Py_END_CRITICAL_SECTION();
3684+
3685+
if (long_cnt == NULL) {
3686+
/* Fast mode: cnt is advanced by count_next()'s lock-free atomic CAS,
3687+
which never takes this critical section, so read it atomically. */
36793688
Py_ssize_t cnt = FT_ATOMIC_LOAD_SSIZE_RELAXED(lz->cnt);
3680-
return PyUnicode_FromFormat("%s(%zd)",
3681-
_PyType_Name(Py_TYPE(lz)), cnt);
3689+
result = PyUnicode_FromFormat("%s(%zd)",
3690+
_PyType_Name(Py_TYPE(lz)), cnt);
36823691
}
3683-
3684-
if (PyLong_Check(lz->long_step)) {
3685-
long step = PyLong_AsLong(lz->long_step);
3686-
if (step == -1 && PyErr_Occurred()) {
3687-
PyErr_Clear();
3688-
}
3689-
if (step == 1) {
3692+
else {
3693+
int hide_step = 0;
3694+
if (PyLong_Check(long_step)) {
3695+
long step = PyLong_AsLong(long_step);
3696+
if (step == -1 && PyErr_Occurred()) {
3697+
PyErr_Clear();
3698+
}
36903699
/* Don't display step when it is an integer equal to 1 */
3691-
return PyUnicode_FromFormat("%s(%R)",
3692-
_PyType_Name(Py_TYPE(lz)),
3693-
lz->long_cnt);
3700+
hide_step = (step == 1);
3701+
}
3702+
if (hide_step) {
3703+
result = PyUnicode_FromFormat("%s(%R)",
3704+
_PyType_Name(Py_TYPE(lz)),
3705+
long_cnt);
3706+
}
3707+
else {
3708+
result = PyUnicode_FromFormat("%s(%R, %R)",
3709+
_PyType_Name(Py_TYPE(lz)),
3710+
long_cnt, long_step);
36943711
}
36953712
}
3696-
return PyUnicode_FromFormat("%s(%R, %R)",
3697-
_PyType_Name(Py_TYPE(lz)),
3698-
lz->long_cnt, lz->long_step);
3713+
3714+
Py_XDECREF(long_cnt);
3715+
Py_XDECREF(long_step);
3716+
return result;
36993717
}
37003718

37013719
static PyType_Slot count_slots[] = {

0 commit comments

Comments
 (0)