From 06b67fb99d603e8e4095564a05a282198775b448 Mon Sep 17 00:00:00 2001 From: Ofey Chan Date: Sun, 2 Oct 2022 11:57:17 +0800 Subject: [PATCH] gh-97591: In `Exception.__setstate__()` acquire strong references before calling `tp_hash` slot (GH-97700) (cherry picked from commit d63943860974f232b5f027dc6535d25d1b4d8fc0) Co-authored-by: Ofey Chan --- Lib/test/test_baseexception.py | 25 +++++++++++++++++++ ...2-10-01-08-55-09.gh-issue-97591.pw6kkH.rst | 2 ++ Objects/exceptions.c | 8 +++++- 3 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2022-10-01-08-55-09.gh-issue-97591.pw6kkH.rst diff --git a/Lib/test/test_baseexception.py b/Lib/test/test_baseexception.py index 8db497a1728507..018626e8db63a5 100644 --- a/Lib/test/test_baseexception.py +++ b/Lib/test/test_baseexception.py @@ -114,6 +114,31 @@ def test_interface_no_arg(self): [repr(exc), exc.__class__.__name__ + '()']) self.interface_test_driver(results) + def test_setstate_refcount_no_crash(self): + # gh-97591: Acquire strong reference before calling tp_hash slot + # in PyObject_SetAttr. + import gc + d = {} + class HashThisKeyWillClearTheDict(str): + def __hash__(self) -> int: + d.clear() + return super().__hash__() + class Value(str): + pass + exc = Exception() + + d[HashThisKeyWillClearTheDict()] = Value() # refcount of Value() is 1 now + + # Exception.__setstate__ should aquire a strong reference of key and + # value in the dict. Otherwise, Value()'s refcount would go below + # zero in the tp_hash call in PyObject_SetAttr(), and it would cause + # crash in GC. + exc.__setstate__(d) # __hash__() is called again here, clearing the dict. + + # This GC would crash if the refcount of Value() goes below zero. + gc.collect() + + class UsageTests(unittest.TestCase): """Test usage of exceptions""" diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-10-01-08-55-09.gh-issue-97591.pw6kkH.rst b/Misc/NEWS.d/next/Core and Builtins/2022-10-01-08-55-09.gh-issue-97591.pw6kkH.rst new file mode 100644 index 00000000000000..d3a5867db7fce2 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2022-10-01-08-55-09.gh-issue-97591.pw6kkH.rst @@ -0,0 +1,2 @@ +Fixed a missing incref/decref pair in `Exception.__setstate__()`. +Patch by Ofey Chan. diff --git a/Objects/exceptions.c b/Objects/exceptions.c index 9639b4436a078b..5dfd1d64d8c1b3 100644 --- a/Objects/exceptions.c +++ b/Objects/exceptions.c @@ -156,8 +156,14 @@ BaseException_setstate(PyObject *self, PyObject *state) return NULL; } while (PyDict_Next(state, &i, &d_key, &d_value)) { - if (PyObject_SetAttr(self, d_key, d_value) < 0) + Py_INCREF(d_key); + Py_INCREF(d_value); + int res = PyObject_SetAttr(self, d_key, d_value); + Py_DECREF(d_value); + Py_DECREF(d_key); + if (res < 0) { return NULL; + } } } Py_RETURN_NONE;