classification
Title: Writing in freed memory in _PyMem_DebugRawRealloc() after shrinking a memory block
Type: crash Stage: patch review
Components: Interpreter Core Versions: Python 3.7, Python 3.6, Python 3.5, Python 3.4
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: davin, haypo, larry, serhiy.storchaka
Priority: critical Keywords: patch

Created on 2017-09-29 06:29 by serhiy.storchaka, last changed 2017-10-05 16:46 by haypo.

Pull Requests
URL Status Linked Edit
PR 3844 open serhiy.storchaka, 2017-10-01 12:51
PR 3898 open haypo, 2017-10-05 16:46
Messages (4)
msg303302 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-09-29 06:29
I tried to build CPython on 64-bit OpenBSD. It was built successfully, but tests crash. They crash on importing hashlib. Actually on compiling hashlib docstring. Minimal reproducer is:

$ ./python -c 'import codecs; codecs.decode(r"\\"*15 + "x"*1906, "unicode-escape")'                  
assertion "maxchar < 128" failed: file "Objects/unicodeobject.c", line 474, function "_PyUnicode_CheckConsistency"
Abort trap (core dumped) 

_PyUnicodeWriter_Finish() calls resize_compact() for truncating the result, but the latter fills the last 15 bytes with \xdb. Since the string is tagged as ASCII, thus crashing in _PyUnicode_CheckConsistency.
msg303307 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-09-29 07:58
"_PyUnicodeWriter_Finish() calls resize_compact() for truncating the result, but the latter fills the last 15 bytes with \xdb. Since the string is tagged as ASCII, thus crashing in _PyUnicode_CheckConsistency."

Cool. My code works as expected to catch bugs :-)
msg303457 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-10-01 12:02
There several bugs in the memory allocator.

Incorrectly detected the case when realloc() resizes a memory block in-place. Wrong address is used for filling the extra memory with DEADBYTE.

-    if (q == oldq && nbytes < original_nbytes) {
+    if (q == oldq - 2*SST && nbytes < original_nbytes) {
         /* shrinking:  mark old extra memory dead */
-        memset(q + nbytes, DEADBYTE, original_nbytes - nbytes);
+        memset(q + 2*SST + nbytes, DEADBYTE, original_nbytes - nbytes);
     }

But fixing this exposes other problem. _PyMem_DebugRawRealloc() is called recursively. _PyMem_DebugRawRealloc calls api->alloc.realloc which is _PyMem_DebugRawRealloc. There are two nested debug allocators. The block is nested in other block, both have their own header and footer.

|header1|header2|------------------------------|footer2|footer1|

_PyMem_DebugRawRealloc fills the extra memory with DEADBYTE.

|header|---------------------------..unused..|footer|
|header|---------------------------|footer|XXXXXXXXX|

But in case of nested _PyMem_DebugRawRealloc's, the outer one (which reallocates the inner block), overwrites the footer of the outer block.

|header1|header2|--------------------..unused..|footer2|footer1|
|header1|header2|--------------------..unused|footer1|XXXXXXXXX| after inner realloc
|header1|header2|--------------------|footer2|YYYYYYYYY|XXXXXXX| after outher realloc

XXX are DEADBYTEs written by the inner allocator, YYY are DEADBYTEs written by the outer allocator.
msg303460 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-10-01 13:02
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 non-trivial system allocators which write an information past the allocated block. This can cause a segfault if unused memory pages are returned to OS.

After creating the PR I have found that it literally restores the code of 2.7 and 3.3. 3.4 and later contain this bug. The bug looks enough serious to me for fixing it in 3.4 and 3.5.
History
Date User Action Args
2017-10-05 16:46:52hayposetpull_requests: + pull_request3870
2017-10-01 13:02:30serhiy.storchakasetnosy: + larry
title: Crash in _PyUnicode_DecodeUnicodeEscape on OpenBSD -> Writing in freed memory in _PyMem_DebugRawRealloc() after shrinking a memory block
messages: + msg303460

versions: + Python 3.4, Python 3.5, Python 3.6
2017-10-01 12:51:55serhiy.storchakasetkeywords: + patch
stage: patch review
pull_requests: + pull_request3825
2017-10-01 12:02:27serhiy.storchakasetpriority: normal -> critical
2017-10-01 12:02:14serhiy.storchakasetmessages: + msg303457
2017-09-29 09:45:52serhiy.storchakasetnosy: + davin
2017-09-29 07:58:08hayposetmessages: + msg303307
2017-09-29 06:52:45serhiy.storchakasettype: crash
2017-09-29 06:29:10serhiy.storchakacreate