Skip to content

Free-threaded: Image.point(list) crashes if another thread mutates the list (getlist / PySequence_Fast, stale size) #9852

Description

@espressolee

getlist() in src/_imaging.c — used by Image.point() — reads a list's size, aliases the list with PySequence_Fast, and walks it with the unchecked PySequence_Fast_GET_ITEM macro over that captured size. On a free-threaded build, another thread shrinking the list mid-walk drives an out-of-bounds read and a segfault. Present on current main.

This is a concrete instance of the already-documented hazard that PySequence_Fast_GET_ITEM / _GET_SIZE do no locking (CPython's free-threading extension docs; numpy/numpy#28046) — not a new class, but I couldn't find this specific getlist/point site reported or fixed (it isn't touched by #9743, which speeds up libImaging/Point.c, nor by the #8389 cleanup), so filing it as another concrete case alongside the thread-safety hardening in #8492 / #9498.

Site (current main, src/_imaging.c)

static UINT8 *getlist(PyObject *arg, Py_ssize_t *length, ...) {
    n = PySequence_Size(arg);                      // 445  size captured once
    if (length && wrong_length && n != *length) ...// 447  point() requires n == 256
    seq = PySequence_Fast(arg, must_be_sequence);  // 458  for a LIST, aliases it (no copy)
    for (i = 0; i < n; i++) {
        op = PySequence_Fast_GET_ITEM(seq, i);     // 465  unchecked macro, stale n
        itemp = PyLong_AsLong(op);                 // dereferences op

Image.point(lut)_point (:1505) → getlist (:1526/1538/1559). If the list is shrunk after line 445's size check, PySequence_Fast_GET_ITEM(seq, i) for i >= new_len reads past the reallocated storage and PyLong_AsLong dereferences a dangling / OOB pointer.

Reproducer

python3.14.0rc1t, Pillow 12.3.0 (official cp314t wheel — GIL is off by default). 12 threads call img.im.point(shared, "L"); 6 threads hard-shrink shared (256→16) and refill. 10 rounds per arm:

arm result
mutate, GIL off (default on 3.14t) 10/10 SIGSEGV
control — no mutator clean 10/10
control — mutate a different list clean 10/10
control — same mutation, PYTHON_GIL=1 clean 10/10

The lut entries must be non-cached ints (> 256): with range(256) every entry is an immortal small int, so a dropped slot still points at a live int and it does not crash even though the index is already out of bounds — "no crash" isn't "safe" here.

ft_pillow.py
"""Pillow 12.3.0 — P3 stale-size in _point/getlist over a caller list (iter/index invalidation).

_imaging.c getlist():
    n = PySequence_Size(arg);                          // 446: length captured
    if (length && wrong_length && n != *length) ...    // 447: must equal expected (256 for point)
    seq = PySequence_Fast(arg, ...);                   // 459: ALIASES the list (no copy)
    for (i = 0; i < n; i++)
        op = PySequence_Fast_GET_ITEM(seq, i);         // 466: unchecked macro, stale n
        itemp = PyLong_AsLong(op);                     // dereferences op

Reached by Image.point (_point, _imaging.c:1505 → getlist at 1526/1538/1559). The lut must be exactly 256 (else ValueError at 447), so the race window is: the list is 256 at the size check, then another thread shrinks it during the 256-iteration walk → PySequence_Fast_GET_ITEM(seq, i>=newsize) is OOB.

LIVE-BY-DEFAULT: Pillow ships a cp314t free-threaded wheel and does not re-enable the GIL on import.
"""
import os, sys, threading, time
from PIL import Image

N = int(os.environ.get("N", 8))
N_MUT = int(os.environ.get("N_MUT", 3))
SECONDS = float(os.environ.get("SECONDS", 5))
MUTATE = os.environ.get("MUTATE", "1") == "1"
DECOY = os.environ.get("DECOY", "0") == "1"

def full():
    # 256 valid lut entries, but HEAP ints (> 256, so not immortal small-int cached):
    # when the list shrinks and drops its ref, these are actually freed → the aliased
    # slot dangles. (range(256) would be immortal small ints and never dangle.)
    return [10_000_000 + i for i in range(256)]

def main():
    gil = sys._is_gil_enabled()
    print(f"py={sys.version.split()[0]} gil={gil} callers={N} mutators={N_MUT if MUTATE else 0} decoy={DECOY}", flush=True)
    shared = full()
    decoy = full()
    core = Image.new("L", (16, 16)).im     # the C ImagingObject; core.point(lut, mode) -> _point
    stop = threading.Event()
    barrier = threading.Barrier(N + (N_MUT if MUTATE else 0) + 1)
    ok = [0] * N

    def caller(t):
        barrier.wait()
        c = 0
        while not stop.is_set():
            try:
                core.point(shared, "L")     # -> _point -> getlist(shared) -> the aliased walk
            except Exception:
                pass                        # ValueError when len != 256 mid-toggle is irrelevant
            c += 1
        ok[t] = c

    def mutator():
        barrier.wait()
        target = decoy if DECOY else shared
        while not stop.is_set():
            # keep it mostly 256 (so the size check passes), then hard-truncate so a
            # call that already passed the check walks a now-shorter, realloc'd list
            # whose freed slots dangle. Refill with FRESH heap ints so the dropped ones
            # are actually released.
            del target[16:]                                  # 256 -> 16 (reallocs down)
            target.extend(10_000_000 + i for i in range(16, 256))  # back to 256, fresh ints

    threads = [threading.Thread(target=caller, args=(t,)) for t in range(N)]
    if MUTATE:
        threads += [threading.Thread(target=mutator) for _ in range(N_MUT)]
    for th in threads: th.start()
    barrier.wait()
    time.sleep(SECONDS)
    stop.set()
    for th in threads: th.join()
    print(f"clean: {sum(ok)} point() calls survived", flush=True)
    return 0

if __name__ == "__main__":
    raise SystemExit(main())

A possible fix

Wrap the walk in Py_BEGIN_CRITICAL_SECTION(arg) (as done for FontObject in #9498), or materialize the items with the bounds-checked PySequence_GetItem / re-read the length each iteration instead of the PySequence_Fast_GET_ITEM macro. The tuple-input path is safe (tuples can't resize); the list path is not.

Not claimed

Thread-safety / robustness, not a security issue — it needs the caller's own code to concurrently mutate the lut. No severity rating; reported so it can be fixed.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions