mirror of
https://gitlab.isc.org/isc-projects/bind9
synced 2025-08-30 14:07:59 +00:00
[9.20] fix: dev: copy __FILE__ when allocating memory
When allocating memory under -m trace|record, the __FILE__ pointer is stored, so it can be printed out later in order to figure out in which file an allocation leaked. (among others, like the line number). However named crashes when called with -m record and using a plugin leaking memory. The reason is that plugins are unloaded earlier than when the leaked allocations are dumped (obviously, as it's done as late as possible). In such circumstances, `__FILE__` is dangling because the dynamically loaded library (the plugin) is not in memory anymore. Fix the crash by systematically copying the `__FILE__` string instead of copying the pointer. Of course, this make each allocation to consume a bit more memory (and longer, as it needs to calculate the length of `__FILE__`) but this occurs only under -m trace|record debugging flags. Backport of MR !10320 Merge branch 'backport-colin-memdump-plugins-9.20' into 'bind-9.20' See merge request isc-projects/bind9!10336
This commit is contained in:
commit
59635e33d0
@ -84,11 +84,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 *file;
|
||||
unsigned int line;
|
||||
const char file[];
|
||||
};
|
||||
|
||||
typedef ISC_LIST(debuglink_t) debuglist_t;
|
||||
@ -200,6 +201,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) {
|
||||
@ -222,14 +232,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->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 +281,7 @@ delete_trace_entry(isc_mem_t *mctx, const void *ptr, size_t size,
|
||||
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);
|
||||
|
@ -369,14 +369,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, 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_destroy(&mctx2);
|
||||
isc_stdio_close(f);
|
||||
@ -391,13 +407,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 */
|
||||
|
Loading…
x
Reference in New Issue
Block a user