From: David van Moolenbroek Date: Wed, 26 Aug 2015 05:57:03 +0000 (+0200) Subject: Fix mmap leak in malloc code upon state transfer X-Git-Url: http://zhaoyanbai.com/repos/%22http:/www.isc.org/icons/zpipe.c?a=commitdiff_plain;h=b7725c855278f39c5e782ceb7724f4aa1864ea14;p=minix.git Fix mmap leak in malloc code upon state transfer The NetBSD libc malloc implementation uses a memory-mapped area for its page directory. Since the process heap is reconstructed upon state transfer for live update, this memory-mapped area must not be transferred to the new process. However, as the new instance of the process being updated inherits all memory-mapped areas of the old instance, it also automatically inherits the malloc implementation's page directory. Thus, we must explicitly free this area in order to avoid a memory leak. The magic pass already detects (de)allocation functions called from within other (de)allocation functions, which is why the mmap(2) and munmap(2) calls of the malloc code are not instrumented as it is. This patch changes that particular case to allow a different hook function to be called for such "nested" allocation calls, for a particular set of nested calls. In particular, the malloc(3) code's mmap(2) and munmap(2) calls are replaced with magic_nested_mmap and magic_nested_munmap calls, respectively. The magic library then tracks memory mapping allocations of the malloc code by providing an implementation for these two wrappers, and frees the allocations upon state transfer. This approach was chosen over various alternatives: - While it appears that nesting could be established by setting a flag while the malloc(3) wrapper is active, and testing the flag in the mmap(2)/munmap(2) wrappers, this approach would fail to detect memory-mapped allocations made from uninstrumented malloc(3) calls, and therefore not a viable option. - It would be possible to obtain the value of the variables that store the information about the memory-mapped area in the malloc code. However, this is rather difficult in practice due to the way the libc malloc implementation stores the size of the are, and it would make the solution more dependent on the specific libc malloc implementation. - It would be possible to use the special "nested" instrumentation for allocations made from certain marked sections. Since we mark the data section of the malloc code already, this would not be hard to do. Switching to this alternative would change very little, and if for any reason this approach yields more advantages in the future, we can still choose to do so. Change-Id: Id977405da86a72458dd10f18e076d8460fd2fb75 --- diff --git a/minix/llvm/include/magic.h b/minix/llvm/include/magic.h index 9bbca57f4..4e55c2ab1 100644 --- a/minix/llvm/include/magic.h +++ b/minix/llvm/include/magic.h @@ -916,6 +916,11 @@ EXTERN void* __stop_magic_functions_st; #define _magic_first_stack_dsentry (_magic_vars->first_stack_dsentry) #define _magic_last_stack_dsentry (_magic_vars->last_stack_dsentry) +/* Magic unmap-memory variables. */ +#ifdef __MINIX +#define _magic_unmap_mem (_magic_vars->unmap_mem) +#endif + /* Magic default stubs. */ EXTERN struct _magic_type magic_default_type; EXTERN struct _magic_dsentry magic_default_dsentry; diff --git a/minix/llvm/include/magic_common.h b/minix/llvm/include/magic_common.h index 7b1048d5b..f2e147ba1 100644 --- a/minix/llvm/include/magic_common.h +++ b/minix/llvm/include/magic_common.h @@ -6,6 +6,7 @@ #define MAGIC_PREFIX_STR "magic_" #define MAGIC_ASR_PREFIX magic_asr_ #define MAGIC_ASR_PREFIX_STR "magic_asr_" +#define MAGIC_NESTED_PREFIX_STR "nested_" #define MAGIC_EVAL_FUNC_PREFIX "me_" #define MAGIC_ANON_MEMBER_PREFIX "magic.anon" #define MAGIC_STRINGREF_HAS_MAGIC_HIDDEN_PREFIX(S) \ @@ -533,11 +534,24 @@ volatile int _magic_var_annotation_ ## V = A #define MAGIC_MEM_FUNC_ALLOC_FLAGS \ MAGIC_MEMA_FUNC_ALLOC_FLAGS MAGIC_MEMA_EXTRA_FUNC_ALLOC_FLAGS, MAGIC_MEMD_FUNC_ALLOC_FLAGS +#ifdef __MINIX +/* Nested allocation functions to hook. That is, functions that are being + * called as part of allocation functions - in particular, malloc - and need to + * be intercepted for tracking purposes - in particular, so that mmap'ed malloc + * page directories can be unmapped in order to avoid memory leaks. MINIX3 only. + */ +#define MAGIC_MEMN_FUNCS \ + __X(mmap), __X(munmap) +#else +#define MAGIC_MEMN_FUNCS "" +#endif + #define MAGIC_DL_FUNCS \ __X(dlopen), __X(dlclose) #define MAGIC_MEMA_FUNC_NAMES MAGIC_MEMA_FUNCS MAGIC_MEMA_EXTRA_FUNCS, "" #define MAGIC_MEMD_FUNC_NAMES MAGIC_MEMD_FUNCS, "" +#define MAGIC_MEMN_FUNC_NAMES MAGIC_MEMN_FUNCS, "" #define MAGIC_MEM_FUNC_NAMES MAGIC_MEM_FUNCS, "" #define MAGIC_DL_FUNC_NAMES MAGIC_DL_FUNCS, "" diff --git a/minix/llvm/include/magic_structs.h b/minix/llvm/include/magic_structs.h index 6634fb927..d7cedd60c 100644 --- a/minix/llvm/include/magic_structs.h +++ b/minix/llvm/include/magic_structs.h @@ -207,6 +207,22 @@ struct _magic_dsodesc { struct _magic_dsodesc *next; }; +/* The following constant is specific to MINIX3; on other platforms, this + * functionality is unused altogether. On MINIX3, the libc malloc code uses + * mmap to create page directories. Since malloc state is discarded upon state + * transfer, we must not instrument its mmap calls in the regular way. On the + * other hand, since mmap'ed regions are transferred to new instances, we end + * up with a memory leak if we do not unmap those mmap'ed regions. Therefore, + * we specifically track the mmap/munmap calls made from the malloc code, and + * explicitly unmap its regions during state transfer. The following constant + * defines how many ranges can be mmap'ed at once. The malloc code uses only + * one page directory, but it may enlarge it by first allocating a new area + * and then unmapping the old one. Therefore, we need two entries. + */ +#ifdef __MINIX +#define MAGIC_UNMAP_MEM_ENTRIES 2 +#endif + /* Magic vars. */ struct _magic_vars_t { @@ -283,6 +299,14 @@ struct _magic_vars_t { int update_dsentry_ranges; int update_dfunction_ranges; +#ifdef __MINIX + /* Memory to unmap after state transfer (MINIX3 only) */ + struct { + void *start; + size_t length; + } unmap_mem[MAGIC_UNMAP_MEM_ENTRIES]; +#endif + /* Range lookup index */ void *sentry_rl_buff; size_t sentry_rl_buff_offset; diff --git a/minix/llvm/passes/include/magic/MagicPass.h b/minix/llvm/passes/include/magic/MagicPass.h index cb5ffa91e..38a65e2e3 100644 --- a/minix/llvm/passes/include/magic/MagicPass.h +++ b/minix/llvm/passes/include/magic/MagicPass.h @@ -90,6 +90,8 @@ class MagicPass : public ModulePass { unsigned getMaxRecursiveSequenceLength(TYPECONST TypeInfo *aTypeInfo); FunctionType* getFunctionType(TYPECONST FunctionType *baseType, std::vector selectedArgs); bool isCompatibleMagicMemFuncType(TYPECONST FunctionType *type, TYPECONST FunctionType* magicType); + Function* findWrapper(Module &M, std::string *magicMemPrefixes, Function *f, std::string fName); + void indexCasts(Module &M, User *U, std::vector &intCastTypes, std::vector &intCastValues, std::map > &bitcastMap); void fillStackInstrumentedFunctions(std::vector &stackIntrumentedFuncs, Function *deepestLLFunction); diff --git a/minix/llvm/passes/include/magic/support/MagicMemFunction.h b/minix/llvm/passes/include/magic/support/MagicMemFunction.h index 7e9cedbbb..091684174 100644 --- a/minix/llvm/passes/include/magic/support/MagicMemFunction.h +++ b/minix/llvm/passes/include/magic/support/MagicMemFunction.h @@ -12,11 +12,12 @@ namespace llvm { class MagicMemFunction { public: - MagicMemFunction(Module &M, Function *function, Function *wrapper, bool isDealloc, int allocFlags); + MagicMemFunction(Module &M, Function *function, Function *wrapper, bool isDealloc, bool isNested, int allocFlags); Function* getFunction() const; Function* getWrapper() const; bool isDeallocFunction() const; + bool isNestedFunction() const; int getAllocFlags() const; Instruction* getInstruction() const; Function* getInstructionParent() const; @@ -45,6 +46,7 @@ private: Function *function; Function *wrapper; bool isDealloc; + bool isNested; int allocFlags; Instruction *instruction; TypeInfo* aTypeInfo; @@ -81,6 +83,8 @@ inline void MagicMemFunction::printDescription(raw_ostream &OS) const { OS << "NULL"; OS << ", isDeallocFunction = "; OS << isDealloc; + OS << ", isNestedFunction = "; + OS << isNested; OS << ", instruction = "; if (instruction) instruction->print(OS); @@ -128,11 +132,12 @@ inline const std::string MagicMemFunction::getDescription() const { return string; } -inline MagicMemFunction::MagicMemFunction(Module &M, Function *function, Function *wrapper, bool isDealloc, int allocFlags) { +inline MagicMemFunction::MagicMemFunction(Module &M, Function *function, Function *wrapper, bool isDealloc, bool isNested, int allocFlags) { this->module = &M; this->function = function; this->wrapper = wrapper; this->isDealloc = isDealloc; + this->isNested = isNested; this->allocFlags = allocFlags; this->instruction = NULL; this->aTypeInfo = NULL; @@ -142,7 +147,7 @@ inline MagicMemFunction::MagicMemFunction(Module &M, Function *function, Functio this->allocNameValue = NULL; this->allocParentNameValue = NULL; assert(function); - if (wrapper && !isDealloc) { + if (wrapper && !isDealloc && !isNested) { lastAllocWrapper = wrapper; } if (isDealloc) { @@ -162,6 +167,10 @@ inline bool MagicMemFunction::isDeallocFunction() const { return isDealloc; } +inline bool MagicMemFunction::isNestedFunction() const { + return isNested; +} + inline int MagicMemFunction::getAllocFlags() const { return allocFlags; } @@ -227,7 +236,7 @@ inline void MagicMemFunction::replaceInstruction(std::map buildWrapper(magicArrayTypePtrMap, voidPtrTypeInfo); } //inject magic args - if (!isDeallocFunction()) { + if (!isDeallocFunction() && !isNestedFunction()) { std::map::iterator it; if (!typeValue) { assert(aTypeInfo); @@ -293,6 +302,7 @@ inline int MagicMemFunction::getMemFunctionPointerParam(Function* function, std: inline void MagicMemFunction::buildWrapper(std::map &magicArrayTypePtrMap, TypeInfo *voidPtrTypeInfo) { assert(!isDeallocFunction()); + assert(!isNestedFunction()); assert(lastAllocWrapper); std::vector ArgTypes; VALUE_TO_VALUE_MAP_TY VMap; diff --git a/minix/llvm/passes/magic/MagicPass.cpp b/minix/llvm/passes/magic/MagicPass.cpp index 3fb9d201d..34f2dafc3 100644 --- a/minix/llvm/passes/magic/MagicPass.cpp +++ b/minix/llvm/passes/magic/MagicPass.cpp @@ -419,6 +419,7 @@ bool MagicPass::runOnModule(Module &M) { #define __X(P) #P std::string magicMemFuncNames[] = { MAGIC_MEM_FUNC_NAMES }; std::string magicMemDeallocFuncNames[] = { MAGIC_MEMD_FUNC_NAMES }; + std::string magicMemNestedFuncNames[] = { MAGIC_MEMN_FUNC_NAMES }; #undef __X int magicMemFuncAllocFlags[] = { MAGIC_MEM_FUNC_ALLOC_FLAGS }; std::string magicMemPrefixes[] = { MAGIC_MEM_PREFIX_STRS }; @@ -433,7 +434,8 @@ bool MagicPass::runOnModule(Module &M) { for(i=0;magicMemFuncNames[i].compare("");i++) { int allocFlags = magicMemFuncAllocFlags[i]; for(unsigned j=0;jgetFunctionType(), w->getFunctionType()) && (w2 = M.getFunction(wName2))) { - w = w2; - wName2.append("_"); - } - if(!isCompatibleMagicMemFuncType(f->getFunctionType(), w->getFunctionType())) { - magicPassErr("Error: wrapper function with incompatible type " << wName << "() found"); - magicPassErr(TypeUtil::getDescription(f->getFunctionType(), MAGIC_TYPE_STR_PRINT_MAX, MAGIC_TYPE_STR_PRINT_MAX_LEVEL) << " != " << TypeUtil::getDescription(w->getFunctionType(), MAGIC_TYPE_STR_PRINT_MAX, MAGIC_TYPE_STR_PRINT_MAX_LEVEL)); - exit(1); - } - bool isDeallocFunction = false; - for(unsigned j=0;magicMemDeallocFuncNames[j].compare("");j++) { - if(!magicMemDeallocFuncNames[j].compare(magicMemFuncNames[i])) { - isDeallocFunction = true; + bool makeNestedFunction = false; + for(unsigned k=0;magicMemNestedFuncNames[k].compare("");k++) { + if (!magicMemNestedFuncNames[k].compare(fName)) { + makeNestedFunction = true; break; } } - MagicMemFunction memFunction(M, f, w, isDeallocFunction, allocFlags); + + Function* w = findWrapper(M, magicMemPrefixes, f, fName); + MagicMemFunction memFunction(M, f, w, isDeallocFunction, false, allocFlags); magicMemFunctions.push_back(memFunction); + if (makeNestedFunction) { + w = findWrapper(M, magicMemPrefixes, f, MAGIC_NESTED_PREFIX_STR + fName); + MagicMemFunction memFunction(M, f, w, isDeallocFunction, true, allocFlags); + magicMemFunctions.push_back(memFunction); + } originalMagicMemFunctions.insert(f); #if DEBUG_ALLOC_LEVEL >= 1 @@ -536,7 +529,7 @@ bool MagicPass::runOnModule(Module &M) { Function *allocWrapper = MagicMemFunction::getCustomWrapper(allocFunction, stdAllocFunc, stdAllocWrapperFunc, allocArgMapping, false); // register the wrapper - MagicMemFunction memFunctionAlloc(M, allocFunction, allocWrapper, false, stdAllocFlags); + MagicMemFunction memFunctionAlloc(M, allocFunction, allocWrapper, false, false, stdAllocFlags); magicMemFunctions.push_back(memFunctionAlloc); originalMagicMemFunctions.insert(allocFunction); #if DEBUG_ALLOC_LEVEL >= 1 @@ -618,7 +611,7 @@ bool MagicPass::runOnModule(Module &M) { exit(1); } Function *blockAllocWrapper = MagicMemFunction::getCustomWrapper(blockAllocFunc, mempoolBlockAllocTemplate, mempoolBlockAllocTemplateWrapper, argMapping, false); - MagicMemFunction memFunctionBlockAlloc(M, blockAllocFunc, blockAllocWrapper, false, mempoolAllocFlags); + MagicMemFunction memFunctionBlockAlloc(M, blockAllocFunc, blockAllocWrapper, false, false, mempoolAllocFlags); mempoolMagicMemFunctions.push_back(memFunctionBlockAlloc); } if (!mempoolMagicMemFunctions.empty()) { // only if the block allocation functions have been successfully processed @@ -981,12 +974,15 @@ bool MagicPass::runOnModule(Module &M) { CS.getCalledValue()) != EqPointers.end())) { bool isDeallocFunction = magicMemFunction.isDeallocFunction(); bool wrapParent = false; + bool isNested = false; TypeInfo *typeInfo = magicVoidTypeInfo; std::string allocName = ""; std::string allocParentName = ""; //check if we have to skip - if(extendedMagicMemFunctions.find(CS.getInstruction()->getParent()->getParent()) != extendedMagicMemFunctions.end()) { - //this call site is only called from some predefined mem function. skip. + //if this call site is only called from some predefined mem function, it is nested + //some function wrappers are for such nested calls, some are not. this must match. + isNested = (extendedMagicMemFunctions.find(CS.getInstruction()->getParent()->getParent()) != extendedMagicMemFunctions.end()); + if (isNested != magicMemFunction.isNestedFunction()) { continue; } if(sbrkFunctions.find(MagicUtil::getCalledFunctionFromCS(CS)) != sbrkFunctions.end()) { @@ -1012,7 +1008,7 @@ bool MagicPass::runOnModule(Module &M) { continue; } //figure out the type and the names - if(!isDeallocFunction) { + if(!isDeallocFunction && !isNested) { int allocCounter = 1; int ret; std::map< std::pair, int>::iterator namesMapIt; @@ -1104,7 +1100,8 @@ bool MagicPass::runOnModule(Module &M) { } if(!magicMemParent && wrapParent) { //if there is no existing parent but we have to wrap the parent, create a parent now and add it to the function queue - MagicMemFunction newMagicMemFunction(M, instructionParent, NULL, false, 0); + assert(!isNested); + MagicMemFunction newMagicMemFunction(M, instructionParent, NULL, false, false, 0); magicMemFunctions.push_back(newMagicMemFunction); magicMemParent = &magicMemFunctions[magicMemFunctions.size()-1]; } @@ -2493,6 +2490,34 @@ bool MagicPass::isCompatibleMagicMemFuncType(TYPECONST FunctionType *type, TYPEC return true; } +Function* MagicPass::findWrapper(Module &M, std::string *magicMemPrefixes, Function *f, std::string fName) +{ + std::string wName, wName2; + Function *w = NULL, *w2 = NULL; + for(unsigned k=0;magicMemPrefixes[k].compare("");k++) { + wName = magicMemPrefixes[k] + fName; + w = M.getFunction(wName); + if(w) { + wName2 = wName + "_"; + break; + } + } + if(!w) { + magicPassErr("Error: no wrapper function found for " << fName << "()"); + exit(1); + } + while(!isCompatibleMagicMemFuncType(f->getFunctionType(), w->getFunctionType()) && (w2 = M.getFunction(wName2))) { + w = w2; + wName2.append("_"); + } + if(!isCompatibleMagicMemFuncType(f->getFunctionType(), w->getFunctionType())) { + magicPassErr("Error: wrapper function with incompatible type " << wName << "() found"); + magicPassErr(TypeUtil::getDescription(f->getFunctionType(), MAGIC_TYPE_STR_PRINT_MAX, MAGIC_TYPE_STR_PRINT_MAX_LEVEL) << " != " << TypeUtil::getDescription(w->getFunctionType(), MAGIC_TYPE_STR_PRINT_MAX, MAGIC_TYPE_STR_PRINT_MAX_LEVEL)); + exit(1); + } + return w; +} + void MagicPass::indexCasts(Module &M, User *U, std::vector &intCastTypes, std::vector &intCastValues, std::map > &bitCastMap) { unsigned i; std::map >::iterator bitCastMapIt; diff --git a/minix/llvm/static/magic/magic.c b/minix/llvm/static/magic/magic.c index 661ff334f..7c34264ff 100644 --- a/minix/llvm/static/magic/magic.c +++ b/minix/llvm/static/magic/magic.c @@ -123,6 +123,10 @@ MAGIC_VAR struct _magic_vars_t _magic_vars_buff = { 1, /* update_dsentry_ranges */ 1, /* update_dfunction_ranges */ +#ifdef __MINIX + { { NULL, 0 } }, /* unmap_mem */ +#endif + NULL, /* sentry_rl_buff */ 0, /* sentry_rl_buff_offset */ 0, /* sentry_rl_buff_size */ diff --git a/minix/llvm/static/magic/magic_mem.c b/minix/llvm/static/magic/magic_mem.c index d22a2f23f..c9a7ac910 100644 --- a/minix/llvm/static/magic/magic_mem.c +++ b/minix/llvm/static/magic/magic_mem.c @@ -2103,5 +2103,67 @@ PUBLIC void *magic_vm_map_cacheblock(__MA_ARGS__ dev_t dev, off_t dev_offset, return data_ptr; } + +/*===========================================================================* + * magic_nested_mmap * + *===========================================================================*/ +void * +magic_nested_mmap(void *start, size_t length, int prot, int flags, + int fd, off_t offset) +{ + void *ptr; + int i; + + ptr = mmap(start, length, prot, flags, fd, offset); + + if (ptr != MAP_FAILED) { + MAGIC_MEM_PRINTF("MAGIC: nested mmap (%p, %zu)\n", ptr, + length); + + /* + * Find a free entry. We do not expect the malloc code to have + * more than two areas mapped at any time. + */ + for (i = 0; i < MAGIC_UNMAP_MEM_ENTRIES; i++) + if (_magic_unmap_mem[i].length == 0) + break; + assert(i < MAGIC_UNMAP_MEM_ENTRIES); + + /* Store the mapping in this entry. */ + _magic_unmap_mem[i].start = ptr; + _magic_unmap_mem[i].length = length; + } + + return ptr; +} + +/*===========================================================================* + * magic_nested_munmap * + *===========================================================================*/ +int +magic_nested_munmap(void *start, size_t length) +{ + int i, r; + + r = munmap(start, length); + + if (r == 0) { + MAGIC_MEM_PRINTF("MAGIC: nested munmap (%p, %zu)\n", start, + length); + + /* Find the corresponding entry. This must always succeed. */ + for (i = 0; i < MAGIC_UNMAP_MEM_ENTRIES; i++) + if (_magic_unmap_mem[i].start == start && + _magic_unmap_mem[i].length == length) + break; + assert(i < MAGIC_UNMAP_MEM_ENTRIES); + + /* Clear the entry. */ + _magic_unmap_mem[i].start = NULL; + _magic_unmap_mem[i].length = 0; + } + + return r; +} #endif diff --git a/minix/llvm/static/magic/magic_st.c b/minix/llvm/static/magic/magic_st.c index 69fcc0dde..1bd9a95e1 100644 --- a/minix/llvm/static/magic/magic_st.c +++ b/minix/llvm/static/magic/magic_st.c @@ -1340,6 +1340,27 @@ PRIVATE void st_vars_clear_ptrs(struct _magic_vars_t *magic_vars) *((void **)(((char *)magic_vars) + offset_list[i])) = NULL; } + +#ifdef __MINIX +PRIVATE void st_unmap_mem(struct _magic_vars_t *magic_vars) +{ + int i, r; + + for (i = 0; i < MAGIC_UNMAP_MEM_ENTRIES; i++) { + if (magic_vars->unmap_mem[i].length != 0) { +#if ST_DEBUG_LEVEL > 0 + printf("st_unmap_mem: unmapping (%p, %zu)\n", + magic_vars->unmap_mem[i].start, + magic_vars->unmap_mem[i].length); +#endif + r = munmap(magic_vars->unmap_mem[i].start, + magic_vars->unmap_mem[i].length); + assert(r == 0); + } + } +} +#endif + PUBLIC int st_init(st_init_info_t *info) { int r, max_buff_sz = 0, dsentries_num; @@ -1435,6 +1456,11 @@ PUBLIC int st_init(st_init_info_t *info) st_init_function_hash(info, _magic_vars); #endif +#ifdef __MINIX + /* Unmap any memory ranges that are not needed in the new process. */ + st_unmap_mem(&st_cached_magic_vars); +#endif + /* Pair metadata entities */ r = pair_metadata_types(info, &st_cached_magic_vars, &st_counterparts, allow_unpaired_types); assert( r == OK && "ERROR occurred during call to pair_metadata_types().");