From 84d97fa73de7060e541231dba8c3c4c717946fd9 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sun, 1 Oct 2017 15:48:16 +0300 Subject: [PATCH 1/3] bpo-31626: Fixed a bug in debug memory allocator. There was a write to freed memory after shrinking a memory block. --- .../2017-10-01-15-48-03.bpo-31626.reLPxY.rst | 2 ++ Objects/obmalloc.c | 16 +++++++++------- 2 files changed, 11 insertions(+), 7 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2017-10-01-15-48-03.bpo-31626.reLPxY.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2017-10-01-15-48-03.bpo-31626.reLPxY.rst b/Misc/NEWS.d/next/Core and Builtins/2017-10-01-15-48-03.bpo-31626.reLPxY.rst new file mode 100644 index 00000000000000..51026a31914844 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2017-10-01-15-48-03.bpo-31626.reLPxY.rst @@ -0,0 +1,2 @@ +Fixed a bug in debug memory allocator. There was a write to freed memory +after shrinking a memory block. diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c index f2651d7574b20f..76649d38826388 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -1460,7 +1460,7 @@ static void * _PyMem_DebugRawRealloc(void *ctx, void *p, size_t nbytes) { debug_alloc_api_t *api = (debug_alloc_api_t *)ctx; - uint8_t *q = (uint8_t *)p, *oldq; + uint8_t *q = (uint8_t *)p; uint8_t *tail; size_t total; /* nbytes + 4*SST */ size_t original_nbytes; @@ -1477,18 +1477,20 @@ _PyMem_DebugRawRealloc(void *ctx, void *p, size_t nbytes) /* overflow: can't represent total as a Py_ssize_t */ return NULL; + if (nbytes < original_nbytes) { + /* shrinking: mark old extra memory dead */ + memset(q + nbytes, DEADBYTE, original_nbytes - nbytes + 2*SST); + } /* Resize and add decorations. We may get a new pointer here, in which * case we didn't get the chance to mark the old memory with DEADBYTE, * but we live with that. */ - oldq = q; q = (uint8_t *)api->alloc.realloc(api->alloc.ctx, q - 2*SST, total); - if (q == NULL) + if (q == NULL) { + if (nbytes <= original_nbytes) { + Py_FatalError("Shrinking reallocation is failed"); + } return NULL; - - if (q == oldq && nbytes < original_nbytes) { - /* shrinking: mark old extra memory dead */ - memset(q + nbytes, DEADBYTE, original_nbytes - nbytes); } write_size_t(q, nbytes); From ccc950121aa423f09124142e882723ea0d015984 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Mon, 2 Oct 2017 13:01:29 +0300 Subject: [PATCH 2/3] Removed Py_FatalError(). --- Objects/obmalloc.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c index 76649d38826388..49c0f0a212ec49 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -1486,12 +1486,8 @@ _PyMem_DebugRawRealloc(void *ctx, void *p, size_t nbytes) * but we live with that. */ q = (uint8_t *)api->alloc.realloc(api->alloc.ctx, q - 2*SST, total); - if (q == NULL) { - if (nbytes <= original_nbytes) { - Py_FatalError("Shrinking reallocation is failed"); - } + if (q == NULL) return NULL; - } write_size_t(q, nbytes); assert(q[SST] == (uint8_t)api->api_id); From e228d0a8c0061b77d31600ab5d7016a8f19e3fdf Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sun, 29 Oct 2017 14:47:54 +0200 Subject: [PATCH 3/3] Don't mark dead bytes in realloc(), --- Objects/obmalloc.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c index 49c0f0a212ec49..1485172102dcc1 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -1477,14 +1477,7 @@ _PyMem_DebugRawRealloc(void *ctx, void *p, size_t nbytes) /* overflow: can't represent total as a Py_ssize_t */ return NULL; - if (nbytes < original_nbytes) { - /* shrinking: mark old extra memory dead */ - memset(q + nbytes, DEADBYTE, original_nbytes - nbytes + 2*SST); - } - /* Resize and add decorations. We may get a new pointer here, in which - * case we didn't get the chance to mark the old memory with DEADBYTE, - * but we live with that. - */ + /* Resize and add decorations. */ q = (uint8_t *)api->alloc.realloc(api->alloc.ctx, q - 2*SST, total); if (q == NULL) return NULL;