From 08447b5deb47e2a0df87fa0a0576d300e5c909b4 Mon Sep 17 00:00:00 2001 From: Konstantin Date: Mon, 31 Jul 2023 09:10:53 +0200 Subject: [PATCH] gh-46376: Return existing pointer when possible in ctypes (#107131) --- Lib/test/test_ctypes/test_keeprefs.py | 27 +++++++++++++++++ ...3-07-24-01-21-16.gh-issue-46376.w-xuDL.rst | 1 + Modules/_ctypes/_ctypes.c | 29 +++++++++++++++++++ 3 files changed, 57 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2023-07-24-01-21-16.gh-issue-46376.w-xuDL.rst diff --git a/Lib/test/test_ctypes/test_keeprefs.py b/Lib/test/test_ctypes/test_keeprefs.py index 23b03b64b4a716..c6fe1de62eae7c 100644 --- a/Lib/test/test_ctypes/test_keeprefs.py +++ b/Lib/test/test_ctypes/test_keeprefs.py @@ -98,6 +98,33 @@ def test_p_cint(self): x = pointer(i) self.assertEqual(x._objects, {'1': i}) + def test_pp_ownership(self): + d = c_int(123) + n = c_int(456) + + p = pointer(d) + pp = pointer(p) + + self.assertIs(pp._objects['1'], p) + self.assertIs(pp._objects['0']['1'], d) + + pp.contents.contents = n + + self.assertIs(pp._objects['1'], p) + self.assertIs(pp._objects['0']['1'], n) + + self.assertIs(p._objects['1'], n) + self.assertEqual(len(p._objects), 1) + + del d + del p + + self.assertIs(pp._objects['0']['1'], n) + self.assertEqual(len(pp._objects), 2) + + del n + + self.assertEqual(len(pp._objects), 2) class PointerToStructure(unittest.TestCase): def test(self): diff --git a/Misc/NEWS.d/next/Library/2023-07-24-01-21-16.gh-issue-46376.w-xuDL.rst b/Misc/NEWS.d/next/Library/2023-07-24-01-21-16.gh-issue-46376.w-xuDL.rst new file mode 100644 index 00000000000000..8e8f0245b4539b --- /dev/null +++ b/Misc/NEWS.d/next/Library/2023-07-24-01-21-16.gh-issue-46376.w-xuDL.rst @@ -0,0 +1 @@ +Prevent memory leak and use-after-free when using pointers to pointers with ctypes diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c index c20d6ae55a06e7..8d4fd30150f19c 100644 --- a/Modules/_ctypes/_ctypes.c +++ b/Modules/_ctypes/_ctypes.c @@ -5129,6 +5129,8 @@ static PyObject * Pointer_get_contents(CDataObject *self, void *closure) { StgDictObject *stgdict; + PyObject *keep, *ptr_probe; + CDataObject *ptr2ptr; if (*(void **)self->b_ptr == NULL) { PyErr_SetString(PyExc_ValueError, @@ -5138,6 +5140,33 @@ Pointer_get_contents(CDataObject *self, void *closure) stgdict = PyObject_stgdict((PyObject *)self); assert(stgdict); /* Cannot be NULL for pointer instances */ + + keep = GetKeepedObjects(self); + if (keep != NULL) { + // check if it's a pointer to a pointer: + // pointers will have '0' key in the _objects + ptr_probe = PyDict_GetItemString(keep, "0"); + + if (ptr_probe != NULL) { + ptr2ptr = (CDataObject*) PyDict_GetItemString(keep, "1"); + if (ptr2ptr == NULL) { + PyErr_SetString(PyExc_ValueError, + "Unexpected NULL pointer in _objects"); + return NULL; + } + // don't construct a new object, + // return existing one instead to preserve refcount + assert( + *(void**) self->b_ptr == ptr2ptr->b_ptr || + *(void**) self->b_value.c == ptr2ptr->b_ptr || + *(void**) self->b_ptr == ptr2ptr->b_value.c || + *(void**) self->b_value.c == ptr2ptr->b_value.c + ); // double-check that we are returning the same thing + Py_INCREF(ptr2ptr); + return (PyObject *) ptr2ptr; + } + } + return PyCData_FromBaseObj(stgdict->proto, (PyObject *)self, 0, *(void **)self->b_ptr);