https://github.com/python/cpython/commit/297f2e093ec95800ae2184330b8408c875523467
commit: 297f2e093ec95800ae2184330b8408c875523467
branch: main
author: Donghee Na <[email protected]>
committer: corona10 <[email protected]>
date: 2024-08-22T23:49:09+09:00
summary:
gh-123083: Fix a potential use-after-free in ``STORE_ATTR_WITH_HINT``
(gh-123092)
files:
A
Misc/NEWS.d/next/Core_and_Builtins/2024-08-17-17-26-25.gh-issue-123083.9xWLJ-.rst
M Lib/test/test_dict.py
M Objects/dictobject.c
M Python/bytecodes.c
M Python/executor_cases.c.h
M Python/generated_cases.c.h
diff --git a/Lib/test/test_dict.py b/Lib/test/test_dict.py
index e5dba7cdc570a8..4030716efb51f9 100644
--- a/Lib/test/test_dict.py
+++ b/Lib/test/test_dict.py
@@ -1476,6 +1476,24 @@ def test_dict_items_result_gc_reversed(self):
gc.collect()
self.assertTrue(gc.is_tracked(next(it)))
+ def test_store_evilattr(self):
+ class EvilAttr:
+ def __init__(self, d):
+ self.d = d
+
+ def __del__(self):
+ if 'attr' in self.d:
+ del self.d['attr']
+ gc.collect()
+
+ class Obj:
+ pass
+
+ obj = Obj()
+ obj.__dict__ = {}
+ for _ in range(10):
+ obj.attr = EvilAttr(obj.__dict__)
+
def test_str_nonstr(self):
# cpython uses a different lookup function if the dict only contains
# `str` keys. Make sure the unoptimized path is used when a non-`str`
diff --git
a/Misc/NEWS.d/next/Core_and_Builtins/2024-08-17-17-26-25.gh-issue-123083.9xWLJ-.rst
b/Misc/NEWS.d/next/Core_and_Builtins/2024-08-17-17-26-25.gh-issue-123083.9xWLJ-.rst
new file mode 100644
index 00000000000000..edc3f1ab6a8e17
--- /dev/null
+++
b/Misc/NEWS.d/next/Core_and_Builtins/2024-08-17-17-26-25.gh-issue-123083.9xWLJ-.rst
@@ -0,0 +1 @@
+Fix a potential use-after-free in ``STORE_ATTR_WITH_HINT``.
diff --git a/Objects/dictobject.c b/Objects/dictobject.c
index 3e9f982ae070a3..a30b3e37319ccf 100644
--- a/Objects/dictobject.c
+++ b/Objects/dictobject.c
@@ -1703,6 +1703,8 @@ insert_split_value(PyInterpreterState *interp,
PyDictObject *mp, PyObject *key,
uint64_t new_version = _PyDict_NotifyEvent(interp,
PyDict_EVENT_MODIFIED, mp, key, value);
STORE_SPLIT_VALUE(mp, ix, Py_NewRef(value));
mp->ma_version_tag = new_version;
+ // old_value should be DECREFed after GC track checking is done, if
not, it could raise a segmentation fault,
+ // when dict only holds the strong reference to value in ep->me_value.
Py_DECREF(old_value);
}
ASSERT_CONSISTENT(mp);
diff --git a/Python/bytecodes.c b/Python/bytecodes.c
index 838af3ee3ab18f..bc418137a9f9e9 100644
--- a/Python/bytecodes.c
+++ b/Python/bytecodes.c
@@ -2235,18 +2235,19 @@ dummy_func(
DEOPT_IF(!DK_IS_UNICODE(dict->ma_keys));
PyDictUnicodeEntry *ep = DK_UNICODE_ENTRIES(dict->ma_keys) + hint;
DEOPT_IF(ep->me_key != name);
+ /* Ensure dict is GC tracked if it needs to be */
+ if (!_PyObject_GC_IS_TRACKED(dict) &&
_PyObject_GC_MAY_BE_TRACKED(PyStackRef_AsPyObjectBorrow(value))) {
+ _PyObject_GC_TRACK(dict);
+ }
old_value = ep->me_value;
PyDict_WatchEvent event = old_value == NULL ? PyDict_EVENT_ADDED :
PyDict_EVENT_MODIFIED;
new_version = _PyDict_NotifyEvent(tstate->interp, event, dict,
name, PyStackRef_AsPyObjectBorrow(value));
ep->me_value = PyStackRef_AsPyObjectSteal(value);
+ dict->ma_version_tag = new_version; // PEP 509
+ // old_value should be DECREFed after GC track checking is done,
if not, it could raise a segmentation fault,
+ // when dict only holds the strong reference to value in
ep->me_value.
Py_XDECREF(old_value);
STAT_INC(STORE_ATTR, hit);
- /* Ensure dict is GC tracked if it needs to be */
- if (!_PyObject_GC_IS_TRACKED(dict) &&
_PyObject_GC_MAY_BE_TRACKED(PyStackRef_AsPyObjectBorrow(value))) {
- _PyObject_GC_TRACK(dict);
- }
- /* PEP 509 */
- dict->ma_version_tag = new_version;
PyStackRef_CLOSE(owner);
}
diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h
index 55b06a0e235dac..4274d51b3fa39c 100644
--- a/Python/executor_cases.c.h
+++ b/Python/executor_cases.c.h
@@ -2637,18 +2637,19 @@
UOP_STAT_INC(uopcode, miss);
JUMP_TO_JUMP_TARGET();
}
+ /* Ensure dict is GC tracked if it needs to be */
+ if (!_PyObject_GC_IS_TRACKED(dict) &&
_PyObject_GC_MAY_BE_TRACKED(PyStackRef_AsPyObjectBorrow(value))) {
+ _PyObject_GC_TRACK(dict);
+ }
old_value = ep->me_value;
PyDict_WatchEvent event = old_value == NULL ? PyDict_EVENT_ADDED :
PyDict_EVENT_MODIFIED;
new_version = _PyDict_NotifyEvent(tstate->interp, event, dict,
name, PyStackRef_AsPyObjectBorrow(value));
ep->me_value = PyStackRef_AsPyObjectSteal(value);
+ dict->ma_version_tag = new_version; // PEP 509
+ // old_value should be DECREFed after GC track checking is done,
if not, it could raise a segmentation fault,
+ // when dict only holds the strong reference to value in
ep->me_value.
Py_XDECREF(old_value);
STAT_INC(STORE_ATTR, hit);
- /* Ensure dict is GC tracked if it needs to be */
- if (!_PyObject_GC_IS_TRACKED(dict) &&
_PyObject_GC_MAY_BE_TRACKED(PyStackRef_AsPyObjectBorrow(value))) {
- _PyObject_GC_TRACK(dict);
- }
- /* PEP 509 */
- dict->ma_version_tag = new_version;
PyStackRef_CLOSE(owner);
stack_pointer += -2;
assert(WITHIN_STACK_BOUNDS());
diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h
index 67bde83e055ede..181940d87fff70 100644
--- a/Python/generated_cases.c.h
+++ b/Python/generated_cases.c.h
@@ -6917,18 +6917,19 @@
DEOPT_IF(!DK_IS_UNICODE(dict->ma_keys), STORE_ATTR);
PyDictUnicodeEntry *ep = DK_UNICODE_ENTRIES(dict->ma_keys) +
hint;
DEOPT_IF(ep->me_key != name, STORE_ATTR);
+ /* Ensure dict is GC tracked if it needs to be */
+ if (!_PyObject_GC_IS_TRACKED(dict) &&
_PyObject_GC_MAY_BE_TRACKED(PyStackRef_AsPyObjectBorrow(value))) {
+ _PyObject_GC_TRACK(dict);
+ }
old_value = ep->me_value;
PyDict_WatchEvent event = old_value == NULL ?
PyDict_EVENT_ADDED : PyDict_EVENT_MODIFIED;
new_version = _PyDict_NotifyEvent(tstate->interp, event, dict,
name, PyStackRef_AsPyObjectBorrow(value));
ep->me_value = PyStackRef_AsPyObjectSteal(value);
+ dict->ma_version_tag = new_version; // PEP 509
+ // old_value should be DECREFed after GC track checking is
done, if not, it could raise a segmentation fault,
+ // when dict only holds the strong reference to value in
ep->me_value.
Py_XDECREF(old_value);
STAT_INC(STORE_ATTR, hit);
- /* Ensure dict is GC tracked if it needs to be */
- if (!_PyObject_GC_IS_TRACKED(dict) &&
_PyObject_GC_MAY_BE_TRACKED(PyStackRef_AsPyObjectBorrow(value))) {
- _PyObject_GC_TRACK(dict);
- }
- /* PEP 509 */
- dict->ma_version_tag = new_version;
PyStackRef_CLOSE(owner);
}
stack_pointer += -2;
_______________________________________________
Python-checkins mailing list -- [email protected]
To unsubscribe send an email to [email protected]
https://mail.python.org/mailman3/lists/python-checkins.python.org/
Member address: [email protected]
