From 859e99374ddd9496cc3acd4705ad6c16cdd87feb Mon Sep 17 00:00:00 2001 From: Ofey Chan Date: Sat, 1 Oct 2022 16:43:14 +0800 Subject: [PATCH 1/4] patch BaseException_setstate and its test. --- Lib/test/test_baseexception.py | 25 +++++++++++++++++++++++++ Objects/exceptions.c | 8 +++++++- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_baseexception.py b/Lib/test/test_baseexception.py index 0061b3fa8e6555..ae646c8dd12827 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/Objects/exceptions.c b/Objects/exceptions.c index 3703fdcda4dbe9..80e98bb4ffa43a 100644 --- a/Objects/exceptions.c +++ b/Objects/exceptions.c @@ -167,8 +167,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; From d86ff33a0f380c0a87704d05b0d7ff2259bf99d9 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Sat, 1 Oct 2022 08:55:10 +0000 Subject: [PATCH 2/4] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2022-10-01-08-55-09.gh-issue-97591.pw6kkH.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2022-10-01-08-55-09.gh-issue-97591.pw6kkH.rst 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..c970669ece5200 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2022-10-01-08-55-09.gh-issue-97591.pw6kkH.rst @@ -0,0 +1 @@ +Fixed a missing incref/decref pair in Exception.__setstate__() From 002844648d3fdff0bd04f776a02f357a71c0e326 Mon Sep 17 00:00:00 2001 From: Ofey Chan Date: Sat, 1 Oct 2022 19:23:41 +0800 Subject: [PATCH 3/4] Patchcheck. --- Lib/test/test_baseexception.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_baseexception.py b/Lib/test/test_baseexception.py index ae646c8dd12827..4c3cf0b964ae56 100644 --- a/Lib/test/test_baseexception.py +++ b/Lib/test/test_baseexception.py @@ -130,7 +130,7 @@ class Value(str): 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 + # 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. From b5dfd41a699a7f13f5ea7963ac696bfdf807dee4 Mon Sep 17 00:00:00 2001 From: Ofey Chan Date: Sun, 2 Oct 2022 09:32:03 +0800 Subject: [PATCH 4/4] Update Misc/NEWS.d/next/Core and Builtins/2022-10-01-08-55-09.gh-issue-97591.pw6kkH.rst Co-authored-by: Guido van Rossum --- .../2022-10-01-08-55-09.gh-issue-97591.pw6kkH.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 index c970669ece5200..d3a5867db7fce2 100644 --- 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 @@ -1 +1,2 @@ -Fixed a missing incref/decref pair in Exception.__setstate__() +Fixed a missing incref/decref pair in `Exception.__setstate__()`. +Patch by Ofey Chan.