[Python-checkins] bpo-36389: _PyObject_IsFreed() now also detects uninitialized memory (GH-12770)
Victor Stinner
webhook-mailer at python.org
Thu Apr 11 05:33:30 EDT 2019
https://github.com/python/cpython/commit/2b00db68554422ec37faba2a80179a0172df6349
commit: 2b00db68554422ec37faba2a80179a0172df6349
branch: master
author: Victor Stinner <vstinner at redhat.com>
committer: GitHub <noreply at github.com>
date: 2019-04-11T11:33:27+02:00
summary:
bpo-36389: _PyObject_IsFreed() now also detects uninitialized memory (GH-12770)
Replace _PyMem_IsFreed() function with _PyMem_IsPtrFreed() inline
function. The function is now way more efficient, it became a simple
comparison on integers, rather than a short loop. It detects also
uninitialized bytes and "forbidden bytes" filled by debug hooks
on memory allocators.
Add unit tests on _PyObject_IsFreed().
files:
M Include/internal/pycore_pymem.h
M Include/pymem.h
M Lib/test/test_capi.py
M Modules/_testcapimodule.c
M Objects/object.c
M Objects/obmalloc.c
diff --git a/Include/internal/pycore_pymem.h b/Include/internal/pycore_pymem.h
index 1e7da87cd75c..78d457d67e82 100644
--- a/Include/internal/pycore_pymem.h
+++ b/Include/internal/pycore_pymem.h
@@ -155,6 +155,31 @@ PyAPI_FUNC(int) _PyMem_SetDefaultAllocator(
PyMemAllocatorDomain domain,
PyMemAllocatorEx *old_alloc);
+/* Heuristic checking if a pointer value is newly allocated
+ (uninitialized) or newly freed. The pointer is not dereferenced, only the
+ pointer value is checked.
+
+ The heuristic relies on the debug hooks on Python memory allocators which
+ fills newly allocated memory with CLEANBYTE (0xCB) and newly freed memory
+ with DEADBYTE (0xDB). Detect also "untouchable bytes" marked
+ with FORBIDDENBYTE (0xFB). */
+static inline int _PyMem_IsPtrFreed(void *ptr)
+{
+ uintptr_t value = (uintptr_t)ptr;
+#if SIZEOF_VOID_P == 8
+ return (value == (uintptr_t)0xCBCBCBCBCBCBCBCB
+ || value == (uintptr_t)0xDBDBDBDBDBDBDBDB
+ || value == (uintptr_t)0xFBFBFBFBFBFBFBFB
+ );
+#elif SIZEOF_VOID_P == 4
+ return (value == (uintptr_t)0xCBCBCBCB
+ || value == (uintptr_t)0xDBDBDBDB
+ || value == (uintptr_t)0xFBFBFBFB);
+#else
+# error "unknown pointer size"
+#endif
+}
+
#ifdef __cplusplus
}
#endif
diff --git a/Include/pymem.h b/Include/pymem.h
index 23457adb5a45..93243f8553b8 100644
--- a/Include/pymem.h
+++ b/Include/pymem.h
@@ -23,8 +23,6 @@ PyAPI_FUNC(int) _PyMem_SetupAllocators(const char *opt);
/* Try to get the allocators name set by _PyMem_SetupAllocators(). */
PyAPI_FUNC(const char*) _PyMem_GetAllocatorsName(void);
-
-PyAPI_FUNC(int) _PyMem_IsFreed(void *ptr, size_t size);
#endif /* !defined(Py_LIMITED_API) */
diff --git a/Lib/test/test_capi.py b/Lib/test/test_capi.py
index 7c68b2c0fc2c..3cd39d4e8b28 100644
--- a/Lib/test/test_capi.py
+++ b/Lib/test/test_capi.py
@@ -526,6 +526,29 @@ def test_pyobject_malloc_without_gil(self):
code = 'import _testcapi; _testcapi.pyobject_malloc_without_gil()'
self.check_malloc_without_gil(code)
+ def check_pyobject_is_freed(self, func):
+ code = textwrap.dedent('''
+ import gc, os, sys, _testcapi
+ # Disable the GC to avoid crash on GC collection
+ gc.disable()
+ obj = _testcapi.{func}()
+ error = (_testcapi.pyobject_is_freed(obj) == False)
+ # Exit immediately to avoid a crash while deallocating
+ # the invalid object
+ os._exit(int(error))
+ ''')
+ code = code.format(func=func)
+ assert_python_ok('-c', code, PYTHONMALLOC=self.PYTHONMALLOC)
+
+ def test_pyobject_is_freed_uninitialized(self):
+ self.check_pyobject_is_freed('pyobject_uninitialized')
+
+ def test_pyobject_is_freed_forbidden_bytes(self):
+ self.check_pyobject_is_freed('pyobject_forbidden_bytes')
+
+ def test_pyobject_is_freed_free(self):
+ self.check_pyobject_is_freed('pyobject_freed')
+
class PyMemMallocDebugTests(PyMemDebugTests):
PYTHONMALLOC = 'malloc_debug'
diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c
index c515efe660b5..1180b4b176e9 100644
--- a/Modules/_testcapimodule.c
+++ b/Modules/_testcapimodule.c
@@ -4236,6 +4236,59 @@ test_pymem_getallocatorsname(PyObject *self, PyObject *args)
}
+static PyObject*
+pyobject_is_freed(PyObject *self, PyObject *op)
+{
+ int res = _PyObject_IsFreed(op);
+ return PyBool_FromLong(res);
+}
+
+
+static PyObject*
+pyobject_uninitialized(PyObject *self, PyObject *args)
+{
+ PyObject *op = (PyObject *)PyObject_Malloc(sizeof(PyObject));
+ if (op == NULL) {
+ return NULL;
+ }
+ /* Initialize reference count to avoid early crash in ceval or GC */
+ Py_REFCNT(op) = 1;
+ /* object fields like ob_type are uninitialized! */
+ return op;
+}
+
+
+static PyObject*
+pyobject_forbidden_bytes(PyObject *self, PyObject *args)
+{
+ /* Allocate an incomplete PyObject structure: truncate 'ob_type' field */
+ PyObject *op = (PyObject *)PyObject_Malloc(offsetof(PyObject, ob_type));
+ if (op == NULL) {
+ return NULL;
+ }
+ /* Initialize reference count to avoid early crash in ceval or GC */
+ Py_REFCNT(op) = 1;
+ /* ob_type field is after the memory block: part of "forbidden bytes"
+ when using debug hooks on memory allocatrs! */
+ return op;
+}
+
+
+static PyObject*
+pyobject_freed(PyObject *self, PyObject *args)
+{
+ PyObject *op = _PyObject_CallNoArg((PyObject *)&PyBaseObject_Type);
+ if (op == NULL) {
+ return NULL;
+ }
+ Py_TYPE(op)->tp_dealloc(op);
+ /* Reset reference count to avoid early crash in ceval or GC */
+ Py_REFCNT(op) = 1;
+ /* object memory is freed! */
+ return op;
+}
+
+
static PyObject*
pyobject_malloc_without_gil(PyObject *self, PyObject *args)
{
@@ -4907,6 +4960,10 @@ static PyMethodDef TestMethods[] = {
{"pymem_api_misuse", pymem_api_misuse, METH_NOARGS},
{"pymem_malloc_without_gil", pymem_malloc_without_gil, METH_NOARGS},
{"pymem_getallocatorsname", test_pymem_getallocatorsname, METH_NOARGS},
+ {"pyobject_is_freed", (PyCFunction)(void(*)(void))pyobject_is_freed, METH_O},
+ {"pyobject_uninitialized", pyobject_uninitialized, METH_NOARGS},
+ {"pyobject_forbidden_bytes", pyobject_forbidden_bytes, METH_NOARGS},
+ {"pyobject_freed", pyobject_freed, METH_NOARGS},
{"pyobject_malloc_without_gil", pyobject_malloc_without_gil, METH_NOARGS},
{"tracemalloc_track", tracemalloc_track, METH_VARARGS},
{"tracemalloc_untrack", tracemalloc_untrack, METH_VARARGS},
diff --git a/Objects/object.c b/Objects/object.c
index bd44acacb615..c9aa479abdc1 100644
--- a/Objects/object.c
+++ b/Objects/object.c
@@ -425,18 +425,17 @@ _Py_BreakPoint(void)
int
_PyObject_IsFreed(PyObject *op)
{
- uintptr_t ptr = (uintptr_t)op;
- if (_PyMem_IsFreed(&ptr, sizeof(ptr))) {
+ if (_PyMem_IsPtrFreed(op) || _PyMem_IsPtrFreed(op->ob_type)) {
return 1;
}
- int freed = _PyMem_IsFreed(&op->ob_type, sizeof(op->ob_type));
- /* ignore op->ob_ref: the value can have be modified
+ /* ignore op->ob_ref: its value can have be modified
by Py_INCREF() and Py_DECREF(). */
#ifdef Py_TRACE_REFS
- freed &= _PyMem_IsFreed(&op->_ob_next, sizeof(op->_ob_next));
- freed &= _PyMem_IsFreed(&op->_ob_prev, sizeof(op->_ob_prev));
+ if (_PyMem_IsPtrFreed(op->_ob_next) || _PyMem_IsPtrFreed(op->_ob_prev)) {
+ return 1;
+ }
#endif
- return freed;
+ return 0;
}
@@ -453,7 +452,7 @@ _PyObject_Dump(PyObject* op)
if (_PyObject_IsFreed(op)) {
/* It seems like the object memory has been freed:
don't access it to prevent a segmentation fault. */
- fprintf(stderr, "<freed object>\n");
+ fprintf(stderr, "<Freed object>\n");
return;
}
diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c
index 1c2a32050f93..e919fad595be 100644
--- a/Objects/obmalloc.c
+++ b/Objects/obmalloc.c
@@ -1914,7 +1914,7 @@ _Py_GetAllocatedBlocks(void)
/* Special bytes broadcast into debug memory blocks at appropriate times.
* Strings of these are unlikely to be valid addresses, floats, ints or
- * 7-bit ASCII.
+ * 7-bit ASCII. If modified, _PyMem_IsPtrFreed() should be updated as well.
*/
#undef CLEANBYTE
#undef DEADBYTE
@@ -2059,22 +2059,6 @@ _PyMem_DebugRawCalloc(void *ctx, size_t nelem, size_t elsize)
}
-/* Heuristic checking if the memory has been freed. Rely on the debug hooks on
- Python memory allocators which fills the memory with DEADBYTE (0xDB) when
- memory is deallocated. */
-int
-_PyMem_IsFreed(void *ptr, size_t size)
-{
- unsigned char *bytes = ptr;
- for (size_t i=0; i < size; i++) {
- if (bytes[i] != DEADBYTE) {
- return 0;
- }
- }
- return 1;
-}
-
-
/* The debug free first checks the 2*SST bytes on each end for sanity (in
particular, that the FORBIDDENBYTEs with the api ID are still intact).
Then fills the original bytes with DEADBYTE.
More information about the Python-checkins
mailing list