Skip to content

Heap corruption in xrandr enumerator: wrong arg passed to XRRGetCrtcInfo #92

@tasticolly

Description

@tasticolly

Heap corruption in xrandr enumerator: wrong arg passed to XRRGetCrtcInfo

Summary

screeninfo/enumerators/xrandr.py calls XRRGetCrtcInfo() with ctypes.byref(output_info) as the second argument. The libXrandr ABI requires XRRScreenResources * there, not XRROutputInfo *. The C function dereferences the wrong pointer and writes past the heap, eventually triggering glibc heap corruption (malloc(): unaligned tcache chunk detected / double linked list corrupted) and SIGABRT.

The bug is silent at low load (a single call may happen to read sensible bytes), but crashes a process reliably under multi-threaded use of get_monitors(). We hit it via camoufox (which depends on screeninfo) in a polling service that launched a Camoufox browser from many parallel workers — the process aborted every ~25 minutes.

Affected versions

  • screeninfo 0.8.1 (PyPI)
  • master as of commit ee3a700
  • Linux x86_64, glibc 2.41, Xvfb / Xorg

Buggy line

screeninfo/enumerators/xrandr.py:

crtc_info = xrandr.XRRGetCrtcInfo(
    display,
    ctypes.byref(output_info),     # ← should be `screen_resources`
    output_info.contents.crtc,
)

screen_resources is the variable already obtained a few lines above via XRRGetScreenResourcesCurrent, and is exactly what libXrandr expects:

XRRCrtcInfo *XRRGetCrtcInfo(Display *dpy, XRRScreenResources *resources, RRCrtc crtc);

(see https://www.x.org/releases/X11R7.7/doc/man/man3/XRRGetCrtcInfo.3.xhtml)

Valgrind evidence

Running our application under valgrind --tool=memcheck produced:

==1== Thread 25:
==1== Invalid read of size 8
==1==    at 0xE3EA242: XextAddDisplay (in /usr/lib/x86_64-linux-gnu/libXext.so.6.4.0)
==1==    by 0xE3E9B62: ??? (in /usr/lib/x86_64-linux-gnu/libXext.so.6.4.0)
==1==    by 0xE3E9E8B: ??? (in /usr/lib/x86_64-linux-gnu/libXext.so.6.4.0)
==1==    by 0xE3EA29A: XextAddDisplay (in /usr/lib/x86_64-linux-gnu/libXext.so.6.4.0)
==1==    by 0xE3D186E: ??? (in /usr/lib/x86_64-linux-gnu/libXrandr.so.2.2.0)
==1==    by 0xE3D55AF: ??? (in /usr/lib/x86_64-linux-gnu/libXrandr.so.2.2.0)
==1==    by 0xE2B86CD: ??? (in /usr/lib/x86_64-linux-gnu/libffi.so.8.1.4)
==1==    by 0xE2B797D: ??? (in /usr/lib/x86_64-linux-gnu/libffi.so.8.1.4)
==1==    by 0xE2B81AA: ffi_call (in /usr/lib/x86_64-linux-gnu/libffi.so.8.1.4)
==1==    by 0xDDDC271: ??? (in /usr/local/lib/python3.11/lib-dynload/_ctypes.cpython-311-x86_64-linux-gnu.so)
==1==    by 0xDDDBC07: ??? (in /usr/local/lib/python3.11/lib-dynload/_ctypes.cpython-311-x86_64-linux-gnu.so)
==1==    by 0x4A0E9D2: _PyObject_MakeTpCall (in /usr/local/lib/libpython3.11.so.1.0)

There are matching Invalid read of size 4 and Invalid write of size 8 reports at the same call site. They appear during the very first iteration where XRRGetCrtcInfo is reached, i.e. as soon as enumerate_monitors() runs once on a display that has at least one connected output.

In production (no valgrind) this manifests as:

malloc(): unaligned tcache chunk detected
Fatal Python error: Aborted

with the crashing thread stuck inside whatever Python code happens to call malloc() next (we initially mis-diagnosed it as curl_cffi, since that was the next thread to allocate).

Reproduction

  1. Linux + a real or virtual X display (Xvfb works).
  2. A package that depends on screeninfo and calls get_monitors() from multiple threads concurrently — e.g. camoufox launching browser contexts from a thread pool. A minimal repro is also possible by calling screeninfo.get_monitors() directly from many threads in a loop.
  3. Wait — the corruption is silent until glibc’s tcache integrity check catches it.

Fix

PR follows: change one line so the right pointer is passed.

                 crtc_info = xrandr.XRRGetCrtcInfo(
                     display,
-                    ctypes.byref(output_info),
+                    screen_resources,
                     output_info.contents.crtc,
                 )

Other concerns in the same file (not part of this PR)

While investigating I noticed two adjacent issues that are worth a separate look but are not the cause of this crash:

  1. None of the xrandr.* / xlib.* symbols have argtypes set, only restype for a few. Without argtypes, ctypes promotes all arguments through int, which on 64-bit Linux silently truncates pointers in the general case (we just got lucky with the calling convention here). Setting explicit argtypes would harden this code against future regressions.
  2. There is no XInitThreads() call before opening displays. XOpenDisplay is called from each invocation of enumerate_monitors() and Xlib is not thread-safe by default; concurrent callers can race on shared per-display state.

Happy to send follow-up PRs for either of these if you’d like.

Thanks for screeninfo — this was a pleasant bug to track down once we got it into valgrind.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions