Skip to content

thread safety issues in unicodeobject.c #153928

Open

Description

@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

Linked PRs

Activity

  1. added
    type-bugAn unexpected behavior, bug, or error
    on Jul 18, 2026
  2. johng commented on Jul 18, 2026

    @johng
    ContributorAuthor

    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++]);

  3. picnixz commented on Jul 18, 2026

    @picnixz
    Member

    With the small test harness (can share)

    Please share as an code block (no files)

  4. devdanzin commented on Jul 18, 2026

    @devdanzin
    Member

    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.

  5. devdanzin commented on Jul 18, 2026

    @devdanzin
    Member

    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.

  6. added a commit that references this issue on Aug 15, 2026
  7. added a commit that references this issue on Oct 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    interpreter-core(Objects, Python, Grammar, and Parser dirs)topic-unicodetype-bugAn unexpected behavior, bug, or error

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions