From a5a5db4b96b395376ac7370430876f3345c49282 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Wed, 1 Nov 2017 11:51:14 +0200 Subject: [PATCH 1/4] bpo-31626: Mark ends of the reallocated block in debug build. Few bytes at the begin and at the end of the reallocated blocks, as well as the header and the trailer, now are erased before calling realloc() in debug build. This will help to detect using or double freeing the reallocated block. --- Objects/obmalloc.c | 58 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 44 insertions(+), 14 deletions(-) diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c index b92116cd554d18..ef1821d89dcca1 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -1517,11 +1517,12 @@ _PyMem_DebugRawRealloc(void *ctx, void *p, size_t nbytes) debug_alloc_api_t *api = (debug_alloc_api_t *)ctx; uint8_t *q; /* base address of malloc'epad d block */ - uint8_t *data; /* p + 2*SST == pointer to data bytes */ + uint8_t *r; uint8_t *tail; /* data + nbytes == pointer to tail pad bytes */ size_t total; /* 2 * SST + nbytes + 2 * SST */ size_t original_nbytes; - int i; +#define ERASED_SIZE 8 + uint8_t save[2*ERASED_SIZE]; /* A copy of erased bytes. */ _PyMem_DebugCheckAddress(api->api_id, p); @@ -1533,31 +1534,60 @@ _PyMem_DebugRawRealloc(void *ctx, void *p, size_t nbytes) } total = nbytes + 4*SST; + tail = q + original_nbytes; + /* Mark the header, the trailer, ERASED_SIZE bytes at the begin and + ERASED_SIZE bytes at the end as dead and save the copy of erased bytes. + */ + if (original_nbytes <= 2*ERASED_SIZE) { + memcpy(save, q, original_nbytes); + memset(q - 2*SST, DEADBYTE, original_nbytes + 4*SST); + } + else { + memcpy(save, q, ERASED_SIZE); + memset(q - 2*SST, DEADBYTE, ERASED_SIZE + 2*SST); + memcpy(&save[ERASED_SIZE], tail - ERASED_SIZE, ERASED_SIZE); + memset(tail - ERASED_SIZE, DEADBYTE, ERASED_SIZE + 2*SST); + } + /* Resize and add decorations. */ - q = (uint8_t *)api->alloc.realloc(api->alloc.ctx, q - 2*SST, total); - if (q == NULL) { - return NULL; + r = (uint8_t *)api->alloc.realloc(api->alloc.ctx, q - 2*SST, total); + if (r != NULL) { + q = r; } bumpserialno(); write_size_t(q, nbytes); - assert(q[SST] == (uint8_t)api->api_id); - for (i = 1; i < SST; ++i) { - assert(q[SST + i] == FORBIDDENBYTE); - } - data = q + 2*SST; + q[SST] = (uint8_t)api->api_id; + memset(q + SST + 1, FORBIDDENBYTE, SST-1); + q += 2*SST; - tail = data + nbytes; + tail = q + nbytes; memset(tail, FORBIDDENBYTE, SST); write_size_t(tail + SST, _PyRuntime.mem.serialno); + /* Restore saved bytes. */ + if (original_nbytes <= 2*ERASED_SIZE) { + memcpy(q, save, Py_MIN(nbytes, original_nbytes)); + } + else { + size_t i = original_nbytes - ERASED_SIZE; + if (nbytes > i) { + memcpy(q + i, &save[ERASED_SIZE], Py_MIN(nbytes - i, ERASED_SIZE)); + } + memcpy(q, save, Py_MIN(nbytes, ERASED_SIZE)); + } + _PyMem_DebugCheckAddress(api->api_id, q); + + if (r == NULL) { + return NULL; + } + if (nbytes > original_nbytes) { /* growing: mark new extra memory clean */ - memset(data + original_nbytes, CLEANBYTE, - nbytes - original_nbytes); + memset(q + original_nbytes, CLEANBYTE, nbytes - original_nbytes); } - return data; + return q; } static void From 148710842b4f7c1da3d38b6580012081a22d1fa4 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sat, 4 Nov 2017 10:04:17 +0200 Subject: [PATCH 2/4] Restore serialno and size when realloc failed. --- Objects/obmalloc.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c index ef1821d89dcca1..9a74e7071b07dc 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -1521,7 +1521,8 @@ _PyMem_DebugRawRealloc(void *ctx, void *p, size_t nbytes) uint8_t *tail; /* data + nbytes == pointer to tail pad bytes */ size_t total; /* 2 * SST + nbytes + 2 * SST */ size_t original_nbytes; -#define ERASED_SIZE 8 + size_t serialno; +#define ERASED_SIZE 16 uint8_t save[2*ERASED_SIZE]; /* A copy of erased bytes. */ _PyMem_DebugCheckAddress(api->api_id, p); @@ -1535,10 +1536,11 @@ _PyMem_DebugRawRealloc(void *ctx, void *p, size_t nbytes) total = nbytes + 4*SST; tail = q + original_nbytes; + serialno = read_size_t(tail + SST); /* Mark the header, the trailer, ERASED_SIZE bytes at the begin and ERASED_SIZE bytes at the end as dead and save the copy of erased bytes. */ - if (original_nbytes <= 2*ERASED_SIZE) { + if (original_nbytes <= sizeof(save)) { memcpy(save, q, original_nbytes); memset(q - 2*SST, DEADBYTE, original_nbytes + 4*SST); } @@ -1551,11 +1553,15 @@ _PyMem_DebugRawRealloc(void *ctx, void *p, size_t nbytes) /* Resize and add decorations. */ r = (uint8_t *)api->alloc.realloc(api->alloc.ctx, q - 2*SST, total); - if (r != NULL) { + if (r == NULL) { + nbytes = original_nbytes; + } + else { q = r; + bumpserialno(); + serialno = _PyRuntime.mem.serialno; } - bumpserialno(); write_size_t(q, nbytes); q[SST] = (uint8_t)api->api_id; memset(q + SST + 1, FORBIDDENBYTE, SST-1); @@ -1563,10 +1569,10 @@ _PyMem_DebugRawRealloc(void *ctx, void *p, size_t nbytes) tail = q + nbytes; memset(tail, FORBIDDENBYTE, SST); - write_size_t(tail + SST, _PyRuntime.mem.serialno); + write_size_t(tail + SST, serialno); /* Restore saved bytes. */ - if (original_nbytes <= 2*ERASED_SIZE) { + if (original_nbytes <= sizeof(save)) { memcpy(q, save, Py_MIN(nbytes, original_nbytes)); } else { From 9f636df7effa900b234937070b975e554076a7ac Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Mon, 6 Nov 2017 15:29:11 +0200 Subject: [PATCH 3/4] Address review comments. --- Objects/obmalloc.c | 65 ++++++++++++++++++++++++++++++---------------- 1 file changed, 42 insertions(+), 23 deletions(-) diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c index 9a74e7071b07dc..8a99747c113469 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -1516,73 +1516,92 @@ _PyMem_DebugRawRealloc(void *ctx, void *p, size_t nbytes) } debug_alloc_api_t *api = (debug_alloc_api_t *)ctx; - uint8_t *q; /* base address of malloc'epad d block */ + uint8_t *head; /* base address of malloc'ed d block */ + uint8_t *data; /* pointer to data bytes */ uint8_t *r; uint8_t *tail; /* data + nbytes == pointer to tail pad bytes */ size_t total; /* 2 * SST + nbytes + 2 * SST */ size_t original_nbytes; size_t serialno; -#define ERASED_SIZE 16 +#define ERASED_SIZE 64 uint8_t save[2*ERASED_SIZE]; /* A copy of erased bytes. */ _PyMem_DebugCheckAddress(api->api_id, p); - q = (uint8_t *)p; - original_nbytes = read_size_t(q - 2*SST); + data = (uint8_t *)p; + head = data - 2*SST; + original_nbytes = read_size_t(head); if (nbytes > (size_t)PY_SSIZE_T_MAX - 4*SST) { /* integer overflow: can't represent total as a Py_ssize_t */ return NULL; } total = nbytes + 4*SST; - tail = q + original_nbytes; + tail = data + original_nbytes; serialno = read_size_t(tail + SST); /* Mark the header, the trailer, ERASED_SIZE bytes at the begin and ERASED_SIZE bytes at the end as dead and save the copy of erased bytes. */ if (original_nbytes <= sizeof(save)) { - memcpy(save, q, original_nbytes); - memset(q - 2*SST, DEADBYTE, original_nbytes + 4*SST); + memcpy(save, data, original_nbytes); + memset(data - 2*SST, DEADBYTE, original_nbytes + 4*SST); } else { - memcpy(save, q, ERASED_SIZE); - memset(q - 2*SST, DEADBYTE, ERASED_SIZE + 2*SST); + memcpy(save, data, ERASED_SIZE); + memset(head, DEADBYTE, ERASED_SIZE + 2*SST); memcpy(&save[ERASED_SIZE], tail - ERASED_SIZE, ERASED_SIZE); memset(tail - ERASED_SIZE, DEADBYTE, ERASED_SIZE + 2*SST); } /* Resize and add decorations. */ - r = (uint8_t *)api->alloc.realloc(api->alloc.ctx, q - 2*SST, total); + r = (uint8_t *)api->alloc.realloc(api->alloc.ctx, head, total); if (r == NULL) { - nbytes = original_nbytes; + write_size_t(head, original_nbytes); + head[SST] = (uint8_t)api->api_id; + memset(head + SST + 1, FORBIDDENBYTE, SST-1); + + memset(tail, FORBIDDENBYTE, SST); + write_size_t(tail + SST, serialno); + + /* Restore saved bytes. */ + if (original_nbytes <= sizeof(save)) { + memcpy(data, save, original_nbytes); + } + else { + memcpy(tail - ERASED_SIZE, &save[ERASED_SIZE], ERASED_SIZE); + memcpy(data, save, ERASED_SIZE); + } + _PyMem_DebugCheckAddress(api->api_id, data); + return NULL; } else { - q = r; + head = r; bumpserialno(); serialno = _PyRuntime.mem.serialno; } - write_size_t(q, nbytes); - q[SST] = (uint8_t)api->api_id; - memset(q + SST + 1, FORBIDDENBYTE, SST-1); - q += 2*SST; + write_size_t(head, nbytes); + head[SST] = (uint8_t)api->api_id; + memset(head + SST + 1, FORBIDDENBYTE, SST-1); + data = head + 2*SST; - tail = q + nbytes; + tail = data + nbytes; memset(tail, FORBIDDENBYTE, SST); write_size_t(tail + SST, serialno); /* Restore saved bytes. */ if (original_nbytes <= sizeof(save)) { - memcpy(q, save, Py_MIN(nbytes, original_nbytes)); + memcpy(data, save, Py_MIN(nbytes, original_nbytes)); } else { size_t i = original_nbytes - ERASED_SIZE; if (nbytes > i) { - memcpy(q + i, &save[ERASED_SIZE], Py_MIN(nbytes - i, ERASED_SIZE)); + memcpy(data + i, &save[ERASED_SIZE], + Py_MIN(nbytes - i, ERASED_SIZE)); } - memcpy(q, save, Py_MIN(nbytes, ERASED_SIZE)); + memcpy(data, save, Py_MIN(nbytes, ERASED_SIZE)); } - _PyMem_DebugCheckAddress(api->api_id, q); + _PyMem_DebugCheckAddress(api->api_id, data); if (r == NULL) { return NULL; @@ -1590,10 +1609,10 @@ _PyMem_DebugRawRealloc(void *ctx, void *p, size_t nbytes) if (nbytes > original_nbytes) { /* growing: mark new extra memory clean */ - memset(q + original_nbytes, CLEANBYTE, nbytes - original_nbytes); + memset(data + original_nbytes, CLEANBYTE, nbytes - original_nbytes); } - return q; + return data; } static void From 381150cbeab98ed3f95fd962b0cd3f01b04bfdaa Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Mon, 6 Nov 2017 18:40:20 +0200 Subject: [PATCH 4/4] Remove code duplications. --- Objects/obmalloc.c | 25 ++++--------------------- 1 file changed, 4 insertions(+), 21 deletions(-) diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c index 8a99747c113469..7f5306f9dcbc38 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -1418,7 +1418,7 @@ static void * _PyMem_DebugRawAlloc(int use_calloc, void *ctx, size_t nbytes) { debug_alloc_api_t *api = (debug_alloc_api_t *)ctx; - uint8_t *p; /* base address of malloc'epad d block */ + uint8_t *p; /* base address of malloc'ed pad block */ uint8_t *data; /* p + 2*SST == pointer to data bytes */ uint8_t *tail; /* data + nbytes == pointer to tail pad bytes */ size_t total; /* 2 * SST + nbytes + 2 * SST */ @@ -1516,7 +1516,7 @@ _PyMem_DebugRawRealloc(void *ctx, void *p, size_t nbytes) } debug_alloc_api_t *api = (debug_alloc_api_t *)ctx; - uint8_t *head; /* base address of malloc'ed d block */ + uint8_t *head; /* base address of malloc'ed pad block */ uint8_t *data; /* pointer to data bytes */ uint8_t *r; uint8_t *tail; /* data + nbytes == pointer to tail pad bytes */ @@ -1556,23 +1556,7 @@ _PyMem_DebugRawRealloc(void *ctx, void *p, size_t nbytes) /* Resize and add decorations. */ r = (uint8_t *)api->alloc.realloc(api->alloc.ctx, head, total); if (r == NULL) { - write_size_t(head, original_nbytes); - head[SST] = (uint8_t)api->api_id; - memset(head + SST + 1, FORBIDDENBYTE, SST-1); - - memset(tail, FORBIDDENBYTE, SST); - write_size_t(tail + SST, serialno); - - /* Restore saved bytes. */ - if (original_nbytes <= sizeof(save)) { - memcpy(data, save, original_nbytes); - } - else { - memcpy(tail - ERASED_SIZE, &save[ERASED_SIZE], ERASED_SIZE); - memcpy(data, save, ERASED_SIZE); - } - _PyMem_DebugCheckAddress(api->api_id, data); - return NULL; + nbytes = original_nbytes; } else { head = r; @@ -1595,13 +1579,12 @@ _PyMem_DebugRawRealloc(void *ctx, void *p, size_t nbytes) } else { size_t i = original_nbytes - ERASED_SIZE; + memcpy(data, save, Py_MIN(nbytes, ERASED_SIZE)); if (nbytes > i) { memcpy(data + i, &save[ERASED_SIZE], Py_MIN(nbytes - i, ERASED_SIZE)); } - memcpy(data, save, Py_MIN(nbytes, ERASED_SIZE)); } - _PyMem_DebugCheckAddress(api->api_id, data); if (r == NULL) { return NULL;