thread safety issues in unicodeobject.c

[johng]

Bug report

Bug description:

Running a TSAN analyser exposed some issues in unicodeobject.c

it->it_index++ is accessed and incremented raw in unicode_ascii_iter_next.

This can result in a crash with the debug build with a small test harness of multiple threads iterating on the same string.

import os
import sys
import threading


ROUNDS = int(sys.argv[1]) if len(sys.argv) > 1 else 4000
LENGTH = 2000
NTHREADS = 8
CHAR = "A"  # the ONLY character that a correct iterator may ever return
# NOEXHAUST>0: each thread stops after that many items, so the iterator is
# never drained to the end -> isolates the it_index race from the it_seq
# double-DECREF-on-exhaustion bug.
NOEXHAUST = int(os.environ.get("NOEXHAUST", "0"))


def drain(it, out, anomalies, barrier, cap):
    barrier.wait()
    n = 0
    while True:
        try:
            c = next(it)
        except StopIteration:
            return
        except Exception as e:  # a corrupted object can raise on use
            anomalies.append(("exception", repr(e)))
            return
        # The corruption checks -- impossible for a correct implementation:
        if type(c) is not str or len(c) != 1 or c != CHAR:
            anomalies.append(("bad-char", repr(c)))
        out.append(1)
        n += 1
        if cap and n >= cap:  # stop BEFORE exhaustion to isolate the it_seq bug
            return


def main():
    total_seen = 0
    total_expected = 0
    dup_skip_rounds = 0
    anomalies = []

    for r in range(ROUNDS):
        s = CHAR * LENGTH
        it = iter(s)
        barrier = threading.Barrier(NTHREADS)
        outs = [[] for _ in range(NTHREADS)]
        threads = [
            threading.Thread(target=drain,
                             args=(it, outs[i], anomalies, barrier, NOEXHAUST))
            for i in range(NTHREADS)
        ]
        for t in threads:
            t.start()
        for t in threads:
            t.join()

        seen = sum(len(o) for o in outs)
        total_seen += seen
        total_expected += LENGTH
        if seen != LENGTH:
            dup_skip_rounds += 1

        if anomalies:
            print(f"[round {r}] CORRUPTION: {anomalies[:5]}")
            break

    print("=" * 60)
    print(f"rounds run            : {r + 1}")
    print(f"chars expected total  : {total_expected}")
    print(f"chars actually seen   : {total_seen}  "
          f"(delta {total_seen - total_expected:+d} = dup/skip)")
    print(f"rounds with dup/skip  : {dup_skip_rounds}")
    print(f"OOB corruptions found : {len(anomalies)}")
    if anomalies:
        print(f"  samples: {anomalies[:10]}")
        print("--> proves the out-of-bounds read: a char not in the source.")
    else:
        print("--> no OOB char observed this run (still UB; TSan flags it "
              "deterministically).")
    return 1 if anomalies else 0


if __name__ == "__main__":
    sys.exit(main())

https://github.com/python/cpython/blob/main/Objects/unicodeobject.c#L14979-L14983

With the small test harness above

Stack (most recent call first):
File "/Users/johng/repos/cpython/str_race_impact.py", line 68 in main
File "/Users/johng/repos/cpython/str_race_impact.py", line 109 in
[1] 59403 abort PYTHON_GIL=0 ./python.exe str_race_impact.py

CPython versions tested on:

CPython main branch

Operating systems tested on:

macOS

[johng]

A similar issue was also discovered in bytesobject

cpython/Objects/bytesobject.c

Lines 3446 to 3448 in bc2fd44

	if (it->it_index < PyBytes_GET_SIZE(seq)) {
	return _PyLong_FromUnsignedChar(
	(unsigned char)seq->ob_sval[it->it_index++]);

[picnixz]

With the small test harness (can share)

Please share as an code block (no files)

[devdanzin]

Reproduced on Linux and confirmed two distinct data races in unicode_ascii_iter_next when a single str iterator is shared across threads. Both are exercised by your harness. Below is a TSan report for the it_index race, crash backtraces for the it_seq race, and a demonstration of the out-of-bounds read returning bytes that are not in the source string.

1. Non-atomic it_index — the race you filed

it->it_index is read (bounds check + PyUnicode_READ) and written (it->it_index++) with no synchronization. TSan flags it deterministically (--disable-gil --with-thread-sanitizer, PYTHON_GIL=0):

WARNING: ThreadSanitizer: data race
  Read of size 8 at 0x… by thread T1:
    #0 unicode_ascii_iter_next   Objects/unicodeobject.c:14979   (it->it_index < len)
    #1 builtin_next              Python/bltinmodule.c:1776
    ...
  Previous write of size 8 at 0x… by thread T8:
    #0 unicode_ascii_iter_next   Objects/unicodeobject.c:14983   (it->it_index++)
    #1 builtin_next              Python/bltinmodule.c:1776

Consequence: an out-of-bounds read. Between the it_index < len check and the PyUnicode_READ(..., it->it_index) the value is re-loaded, so a sibling thread can push it_index past the end in that window, and the read then addresses memory outside the string. On a free-threaded build compiled at -O0 (so the field is re-read rather than kept in a register), iterating an all-"A" string returns characters that are not in the source:

next() returned '\x00'   <- data[len], the NUL terminator
next() returned '\x05'   <- data[len+N], a byte from the heap PAST the string's allocation
next() returned '\x07'   <-  "  "

'\x00' is the terminator, but '\x05'/'\x07' are read from beyond the object's header + len + 1 allocation — i.e. unrelated heap memory. The overshoot depth is bounded by how many threads clear the it_index < len check before it_index crosses len; because the non-atomic ++ also loses increments, threads stay clustered near the same index and pile up at the boundary, so a longer string produces a deeper overshoot. There is also a second latent OOB in the same path: the return is &_Py_SINGLETON(strings).ascii[chr], an array of only 128 entries, so any leaked byte ≥ 128 indexes past that array and yields a bogus PyObject *.

2. it_seq double-DECREF on exhaustion — a use-after-free (crashes)

The exhaustion branch is also unsynchronized:

PyObject *seq = it->it_seq;
if (seq == NULL) { return NULL; }
...
if (it->it_index < len) { ...; return item; }
it->it_seq = NULL;
Py_DECREF(seq);          // <-- two threads can both reach here with the same non-NULL seq
return NULL;

Two threads that both read a non-NULL seq and both fall through to the exhaustion branch each run Py_DECREF(seq) → the string is decref'd twice → freed early → use-after-free. This is what aborts your default (drain-to-exhaustion) run.

Crash backtrace (free-threaded + ASan). A concurrent unicode_ascii_iter_next dereferences the freed seq — note rax = 0xddddddddddddde95, CPython's freed-memory fill:

AddressSanitizer: SEGV ... caused by a READ memory access
  rax = 0xddddddddddddde95   (0xdd = freed-memory poison)
    #0 PyType_HasFeature            Include/object.h:755
    #1 PyUnicode_IS_COMPACT_ASCII   Include/cpython/unicodeobject.h:239
    #2 unicode_ascii_iter_next      Objects/unicodeobject.c:14978
    #3 _PyForIter_VirtualIteratorNext Python/ceval.c:3775           (FOR_ITER)
    ...
    #20 thread_run                  Modules/_threadmodule.c:388
SUMMARY: AddressSanitizer: SEGV Include/object.h:755 in PyType_HasFeature

On a --with-pydebug build the same UAF is caught a step earlier by the consistency check (the freed seq is no longer a compact-ASCII string):

Objects/unicodeobject.c:612: _PyUnicode_CheckConsistency: Assertion failed: kind == PyUnicode_1BYTE_KIND
  #9  _PyUnicode_CheckConsistency   Objects/unicodeobject.c:612
  #10 unicode_ascii_iter_next       Objects/unicodeobject.c:14977
  #11 _PyForIter_VirtualIteratorNext Python/ceval.c:3775            (FOR_ITER)

The non-fast-path unicodeiter_next (line ~14959) has the identical it->it_seq = NULL; Py_DECREF(seq); exhaustion shape, so non-ASCII strings look affected too.

Reproduction

• Build: ./configure --disable-gil [--with-thread-sanitizer | --with-pydebug]; the OOB-byte / UAF-SEGV demos want optimizations off (CFLAGS='-O0 -g', NDEBUG) so it_index/seq are re-read from memory rather than cached in a register.
• Run: PYTHON_GIL=0 [TSAN_OPTIONS=halt_on_error=1] ./python <harness>; the it_index race needs no exhaustion (NOEXHAUST>0), the it_seq UAF needs drain-to-exhaustion.

Fix direction

Both are the "non-atomic access to a shared object" free-threading class: the reader should use the same atomic accessor the writer does. Concretely, it_index wants an atomic load/store (or a per-iterator critical section), and the exhaustion path should publish it_seq = NULL atomically and Py_DECREF only the thread that won the exchange (e.g. PyObject *seq = _Py_atomic_exchange_ptr(&it->it_seq, NULL); if (seq) Py_DECREF(seq);), so the string is released exactly once.

---

Reproductions and this write-up were produced with Claude Code; happy to share the exact harnesses/build flags.

[devdanzin]

Confirming the bytes case on Linux (--disable-gil --with-thread-sanitizer), and flagging two things beyond the it_index lines above:

1. striter_next has the same it_seq double-DECREF as the str iterator. The exhaustion branch just below the lines you linked —

it->it_seq = NULL;
Py_DECREF(seq);      // Objects/bytesobject.c ~:3450

— is reached with no synchronization, so two threads sharing one bytes iterator that both exhaust it Py_DECREF the same bytes twice → refcount underflow / use-after-free (the crash-capable one, matching the str it_seq race). TSan on the plain it_index race first:

WARNING: ThreadSanitizer: data race
  Read  of size 8  striter_next  Objects/bytesobject.c:3446   (it->it_index < PyBytes_GET_SIZE)
  Prev write of 8  striter_next  Objects/bytesobject.c:3448   (ob_sval[it->it_index++])

2. It's the whole sequence-iterator family, not just str + bytes. The it->it_index (non-atomic cursor) + it->it_seq = NULL; Py_DECREF(seq) (exhaustion) shape is shared by the list, tuple, and range iterators as well; a fuzzer run also caught the dict iterator racing in dictiter_iternext_threadsafe. Worth fixing the family together (atomic it_index; release it_seq once via an atomic exchange) rather than per-type.