[issue31626] Writing in freed memory in _PyMem_DebugRawRealloc() after shrinking a memory block

Serhiy Storchaka added the comment: > * 3.6: don't memset() --> correct, but don't detect "use after free" bug > when a memory block is skrinked And doesn't detect "use after free" bug when a memory block is expanded. And 2.7 doesn't detect this. Only 3.7 detects

[issue31626] Writing in freed memory in _PyMem_DebugRawRealloc() after shrinking a memory block

Change by STINNER Victor : -- versions: +Python 2.7 ___ Python tracker ___ ___

[issue31626] Writing in freed memory in _PyMem_DebugRawRealloc() after shrinking a memory block

STINNER Victor added the comment: Ok, let me summarize: * 2.7: memset() *before* realloc() if shrinking a memory block, but crash when Py_FatalError() if realloc() fails on shrinking --> cannot corrupt memory, but can crash if realloc() fails on shrinking * master

[issue31626] Writing in freed memory in _PyMem_DebugRawRealloc() after shrinking a memory block

Serhiy Storchaka added the comment: I prefer to keep the current code as is. -- ___ Python tracker ___

[issue31626] Writing in freed memory in _PyMem_DebugRawRealloc() after shrinking a memory block

STINNER Victor added the comment: Serhiy: I added a Py_FatalError() to Python 2.7 if the case that must not happen does happen. Are you ok to apply the same change for Python 3.6 (commit ed4743a2f2bb8d88f7f1aa9307794be9a44f1e0f), or do you prefer to backport your

[issue31626] Writing in freed memory in _PyMem_DebugRawRealloc() after shrinking a memory block

STINNER Victor added the comment: New changeset ed4743a2f2bb8d88f7f1aa9307794be9a44f1e0f by Victor Stinner in branch '2.7': bpo-31626: Fix _PyObject_DebugReallocApi() (#4310) https://github.com/python/cpython/commit/ed4743a2f2bb8d88f7f1aa9307794be9a44f1e0f

[issue31626] Writing in freed memory in _PyMem_DebugRawRealloc() after shrinking a memory block

STINNER Victor added the comment: For allocations larger than 512 bytes, PyObject_Malloc() calls PyMem_RawMalloc(). When debug hooks are installed, PyObject_Malloc() calls a debug hook which calls PyMem_RawMalloc() which calls another debug hook. --

[issue31626] Writing in freed memory in _PyMem_DebugRawRealloc() after shrinking a memory block

Serhiy Storchaka added the comment: Yet one issue is left. In debug mode two debug allocators are used, the one is nested in the other. Is it correct? -- ___ Python tracker

[issue31626] Writing in freed memory in _PyMem_DebugRawRealloc() after shrinking a memory block

Change by STINNER Victor : -- pull_requests: +4271 ___ Python tracker ___ ___

[issue31626] Writing in freed memory in _PyMem_DebugRawRealloc() after shrinking a memory block

Serhiy Storchaka added the comment: There are large discussions at PR 3844 and PR 4210. -- ___ Python tracker ___

[issue31626] Writing in freed memory in _PyMem_DebugRawRealloc() after shrinking a memory block

Serhiy Storchaka added the comment: New changeset 3cc4c53a64bdcf21431ad306eca0e568f88735a2 by Serhiy Storchaka in branch 'master': bpo-31626: Mark ends of the reallocated block in debug build. (#4210)

[issue31626] Writing in freed memory in _PyMem_DebugRawRealloc() after shrinking a memory block

Change by Serhiy Storchaka : -- pull_requests: +4179 ___ Python tracker ___ ___

[issue31626] Writing in freed memory in _PyMem_DebugRawRealloc() after shrinking a memory block

STINNER Victor added the comment: While the commit b484d5606ca76f9bbd0f5de7a6ef753400213e94 fixes a crash on OpenBSD and makes the code "correct" on any platforms, I still see it as a feature regression. Detecting buffer overflow and using freed memory are feature

[issue31626] Writing in freed memory in _PyMem_DebugRawRealloc() after shrinking a memory block

Change by Serhiy Storchaka : -- priority: critical -> normal ___ Python tracker ___

[issue31626] Writing in freed memory in _PyMem_DebugRawRealloc() after shrinking a memory block

Larry Hastings added the comment: > Most, if not all, calls to _PyMem_DebugRawRealloc() are protected by > the GIL. If there is a single thread using the memory block, > I think that it's perfectly fine to write after it's deallocated. I don't quite follow where the

[issue31626] Writing in freed memory in _PyMem_DebugRawRealloc() after shrinking a memory block

Serhiy Storchaka added the comment: I have removed the incorrect code in master and 3.6, this unblocks testing debug build on OpenBSD. I don't think it is worth to backport this change to 3.5 and 3.4, since the bug affects only debug build. Here is a patch which

[issue31626] Writing in freed memory in _PyMem_DebugRawRealloc() after shrinking a memory block

Serhiy Storchaka added the comment: New changeset ece5659565e083baaee4d185ce181a98aaee7f96 by Serhiy Storchaka (Miss Islington (bot)) in branch '3.6': bpo-31626: Fixed a bug in debug memory allocator. (GH-3844) (#4191)

[issue31626] Writing in freed memory in _PyMem_DebugRawRealloc() after shrinking a memory block

Change by Roundup Robot : -- pull_requests: +4161 ___ Python tracker ___

[issue31626] Writing in freed memory in _PyMem_DebugRawRealloc() after shrinking a memory block

Serhiy Storchaka added the comment: New changeset b484d5606ca76f9bbd0f5de7a6ef753400213e94 by Serhiy Storchaka in branch 'master': bpo-31626: Fixed a bug in debug memory allocator. (#3844)

[issue31626] Writing in freed memory in _PyMem_DebugRawRealloc() after shrinking a memory block

Stefan Krah added the comment: Ah sorry, you mean it cannot write the special bytes. -- ___ Python tracker ___

[issue31626] Writing in freed memory in _PyMem_DebugRawRealloc() after shrinking a memory block

Stefan Krah added the comment: > realloc() must not touch the original buffer on failure I don't understand this: If realloc() fails, the original buffer is perfectly valid. -- nosy: +skrah ___ Python tracker

[issue31626] Writing in freed memory in _PyMem_DebugRawRealloc() after shrinking a memory block

Change by Xavier de Gaye : -- nosy: +xdegaye ___ Python tracker ___ ___ Python-bugs-list

[issue31626] Writing in freed memory in _PyMem_DebugRawRealloc() after shrinking a memory block

Serhiy Storchaka added the comment: The current code OBVIOUSLY is wrong. Bytes are erased if q == oldq && nbytes < original_nbytes. But q == oldq only if realloc() returns the new address 2*sizeof(size_t) bytes larger than its argument. This is virtually never

[issue31626] Writing in freed memory in _PyMem_DebugRawRealloc() after shrinking a memory block

STINNER Victor added the comment: "PR 4119 is too complex for a bugfix (especially if backport it to 3.5 and 3.4). It can introduce other regressions." Which kind of regression do you expect? Something like a crash? "The performance hit is not the only issue.

[issue31626] Writing in freed memory in _PyMem_DebugRawRealloc() after shrinking a memory block

STINNER Victor added the comment: "the bug should be fixed in all affected versions" I don't understand why you insist to change Python 3.4 and Python 3.5. IMHO this issue only impacts OpenBSD. "The current code in not correct on all platforms. We don't know how

[issue31626] Writing in freed memory in _PyMem_DebugRawRealloc() after shrinking a memory block

Serhiy Storchaka added the comment: In this case it would be safe to not erase bytes at all. PR 4119 is too complex for a bugfix (especially if backport it to 3.5 and 3.4). It can introduce other regressions. The performance hit is not the only issue. Allocating

[issue31626] Writing in freed memory in _PyMem_DebugRawRealloc() after shrinking a memory block

STINNER Victor added the comment: > The current code in not correct on all platforms. My PR 4119 only changes the behaviour on OpenBSD, but I'm not sure about that. Maybe it's simpler to apply this fix on all platforms, as I asked on the PR itself? --

[issue31626] Writing in freed memory in _PyMem_DebugRawRealloc() after shrinking a memory block

STINNER Victor added the comment: > PR 3844 restores the behavior of 2.7 and 3.3. I propose to merge it first, > and develop other enhancements later. Please don't. This PR reintroduced a bug that I fixed in bpo-18408: commit c4266360fc70745d49b09f2c29cda91c1a007525

[issue31626] Writing in freed memory in _PyMem_DebugRawRealloc() after shrinking a memory block

Serhiy Storchaka added the comment: The current code in not correct on all platforms. We don't know how many of random bugs, hangs and crashes on other platforms are caused by this bug. I'm not surprised that it was caught on OpenBSD since the robustness and

[issue31626] Writing in freed memory in _PyMem_DebugRawRealloc() after shrinking a memory block

STINNER Victor added the comment: "I tried to build CPython on 64-bit OpenBSD. It was built successfully, but tests crash." It's the first time that anyone complains about _PyMem_DebugRawRealloc(). The behaviour seems to be very specific to OpenBSD. Even if the

[issue31626] Writing in freed memory in _PyMem_DebugRawRealloc() after shrinking a memory block

Change by STINNER Victor : -- pull_requests: +4089 ___ Python tracker ___ ___

[issue31626] Writing in freed memory in _PyMem_DebugRawRealloc() after shrinking a memory block

STINNER Victor added the comment: > Using nested _PyMem_DebugRawRealloc() looks suspicions to me. This may be a > bug. PyObject_Malloc() calls PyMem_RawMalloc() for allocations larger than 512 bytes. When debug hooks are enabled, PyObject_Malloc() and

[issue31626] Writing in freed memory in _PyMem_DebugRawRealloc() after shrinking a memory block

Serhiy Storchaka added the comment: Ping. -- ___ Python tracker ___ ___

[issue31626] Writing in freed memory in _PyMem_DebugRawRealloc() after shrinking a memory block

Change by STINNER Victor : -- pull_requests: +3870 ___ Python tracker ___ ___

[issue31626] Writing in freed memory in _PyMem_DebugRawRealloc() after shrinking a memory block

Serhiy Storchaka added the comment: Using nested _PyMem_DebugRawRealloc() looks suspicions to me. This may be a bug. But even without nested _PyMem_DebugRawRealloc() writing to the extra memory after using realloc() looks wrong to me. This can break other