diff --git a/lib/isc/mem.c b/lib/isc/mem.c index 1b569f042d..22ad423f7e 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -83,12 +83,12 @@ volatile void *isc__mem_malloc = mallocx; #if ISC_MEM_TRACKLINES typedef struct debuglink debuglink_t; struct debuglink { + size_t dlsize; ISC_LINK(debuglink_t) link; const void *ptr; size_t size; - const char *func; - const char *file; unsigned int line; + const char file[]; }; typedef ISC_LIST(debuglink_t) debuglist_t; @@ -198,6 +198,15 @@ add_trace_entry(isc_mem_t *mctx, const void *ptr, size_t size FLARG) { uint32_t hash; uint32_t idx; + /* + * "file" needs to be copied because it can be part of a dynamically + * loaded plugin which would be unloaded at the time the trace is + * dumped. Storing "file" pointer then leads to a dangling pointer + * dereference and a crash. + */ + size_t filelen = strlen(file) + 1; + size_t dlsize = STRUCT_FLEX_SIZE(dl, file, filelen); + MCTXLOCK(mctx); if ((mctx->debugging & ISC_MEM_DEBUGTRACE) != 0) { @@ -221,15 +230,15 @@ add_trace_entry(isc_mem_t *mctx, const void *ptr, size_t size FLARG) { #endif idx = hash % DEBUG_TABLE_COUNT; - dl = mallocx(sizeof(debuglink_t), mctx->jemalloc_flags); + dl = mallocx(dlsize, mctx->jemalloc_flags); INSIST(dl != NULL); ISC_LINK_INIT(dl, link); dl->ptr = ptr; dl->size = size; - dl->func = func; - dl->file = file; dl->line = line; + dl->dlsize = dlsize; + strlcpy((char *)dl->file, file, filelen); ISC_LIST_PREPEND(mctx->debuglist[idx], dl, link); mctx->debuglistcnt++; @@ -270,7 +279,7 @@ delete_trace_entry(isc_mem_t *mctx, const void *ptr, size_t size FLARG) { while (dl != NULL) { if (dl->ptr == ptr) { ISC_LIST_UNLINK(mctx->debuglist[idx], dl, link); - sdallocx(dl, sizeof(*dl), mctx->jemalloc_flags); + sdallocx(dl, dl->dlsize, mctx->jemalloc_flags); goto unlock; } dl = ISC_LIST_NEXT(dl, link); diff --git a/tests/isc/mem_test.c b/tests/isc/mem_test.c index aaf94c0d77..31a6364834 100644 --- a/tests/isc/mem_test.c +++ b/tests/isc/mem_test.c @@ -370,14 +370,30 @@ ISC_RUN_TEST_IMPL(isc_mem_recordflag) { char buf[4096], *p; FILE *f; void *ptr; + void *ptr2; + char dummyfilename[2] = "a"; result = isc_stdio_open("mem.output", "w", &f); assert_int_equal(result, ISC_R_SUCCESS); isc_mem_create(&mctx2); + ptr = isc_mem_get(mctx2, 2048); assert_non_null(ptr); + + /* + * This strange allocation verifies that the file name (dummyfilename, + * instead of __FILE__) actually gets copied instead of simply putting + * its pointers in the debuglink struct. This avoids named to crash on + * shutdown if a plugin leaked memory, because the plugin would be + * unloaded, and __FILE__ pointer passed at this time would be dangling. + */ + ptr2 = isc__mem_get(mctx2, 1024, 0, __func__, dummyfilename, __LINE__); + assert_non_null(ptr2); + dummyfilename[0] = 'b'; + isc__mem_printactive(mctx2, f); + isc_mem_put(mctx2, ptr2, 1024); isc_mem_put(mctx2, ptr, 2048); isc_mem_detach(&mctx2); isc_stdio_close(f); @@ -392,13 +408,20 @@ ISC_RUN_TEST_IMPL(isc_mem_recordflag) { buf[sizeof(buf) - 1] = 0; - p = strchr(buf, '\n'); + /* + * Find the allocation of ptr2 and make sure it contains + * "[...] 1024 file a line [...]" and _not_ "[...] 1024 file b [...]", + * which prove the copy + */ + p = strstr(buf, "1024 file a line"); assert_non_null(p); - assert_in_range(p, 0, buf + sizeof(buf) - 3); - assert_memory_equal(p + 2, "ptr ", 4); - p = strchr(p + 1, '\n'); + + /* + * Find the allocation of ptr and make sure it contains "[...] 2048 file + * mem_test.c line [...]" + */ + p = strstr(buf, "2048 file mem_test.c line"); assert_non_null(p); - assert_int_equal(strlen(p), 1); } /* test mem with trace flag */