]> git.ipfire.org Git - thirdparty/Python/cpython.git/commitdiff
gh-119999: Fix potential race condition in `_Py_ExplicitMergeRefcount` (#120000)
authorSam Gross <colesbury@gmail.com>
Tue, 4 Jun 2024 19:26:26 +0000 (15:26 -0400)
committerGitHub <noreply@github.com>
Tue, 4 Jun 2024 19:26:26 +0000 (15:26 -0400)
We need to write to `ob_ref_local` and `ob_tid` before `ob_ref_shared`.
Once we mark `ob_ref_shared` as merged, some other thread may free the
object because the caller also passes in `-1` as `extra` to give up its
only reference.

Objects/object.c

index 2e9962f4651e1cfa24f4e66b09e1cafc6515d250..b7730475ac3768ca3ec378ac4db8488eb8c1eb3c 100644 (file)
@@ -401,24 +401,27 @@ Py_ssize_t
 _Py_ExplicitMergeRefcount(PyObject *op, Py_ssize_t extra)
 {
     assert(!_Py_IsImmortal(op));
+
+#ifdef Py_REF_DEBUG
+    _Py_AddRefTotal(_PyThreadState_GET(), extra);
+#endif
+
+    // gh-119999: Write to ob_ref_local and ob_tid before merging the refcount.
+    Py_ssize_t local = (Py_ssize_t)op->ob_ref_local;
+    _Py_atomic_store_uint32_relaxed(&op->ob_ref_local, 0);
+    _Py_atomic_store_uintptr_relaxed(&op->ob_tid, 0);
+
     Py_ssize_t refcnt;
     Py_ssize_t new_shared;
     Py_ssize_t shared = _Py_atomic_load_ssize_relaxed(&op->ob_ref_shared);
     do {
         refcnt = Py_ARITHMETIC_RIGHT_SHIFT(Py_ssize_t, shared, _Py_REF_SHARED_SHIFT);
-        refcnt += (Py_ssize_t)op->ob_ref_local;
+        refcnt += local;
         refcnt += extra;
 
         new_shared = _Py_REF_SHARED(refcnt, _Py_REF_MERGED);
     } while (!_Py_atomic_compare_exchange_ssize(&op->ob_ref_shared,
                                                 &shared, new_shared));
-
-#ifdef Py_REF_DEBUG
-    _Py_AddRefTotal(_PyThreadState_GET(), extra);
-#endif
-
-    _Py_atomic_store_uint32_relaxed(&op->ob_ref_local, 0);
-    _Py_atomic_store_uintptr_relaxed(&op->ob_tid, 0);
     return refcnt;
 }
 #endif  /* Py_GIL_DISABLED */