From patchwork Mon Apr 8 22:06:29 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Carlos O'Donell X-Patchwork-Id: 1081550 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=sourceware.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=libc-alpha-return-101241-incoming=patchwork.ozlabs.org@sourceware.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.b="eThC6S2k"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 44dPfQ3T86z9sR0 for ; Tue, 9 Apr 2019 08:07:10 +1000 (AEST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:to:from:subject:message-id:date:mime-version :content-type:content-transfer-encoding; q=dns; s=default; b=b04 FAZxkNmbyCzXPyKl+skeeX5yX8ZUTCN4x7FftAkuHDc5Pho+QMN8+e0jIK3q6op9 ncPn+OekadrBhukgTDiWlV2/Iww4545/vi1Fm6M3S27Ar5HO3mY1uHXwN0euPRK3 QANgTh+xxL25kysUDRCnHWXEu53Ij7a2tO69v8cA= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:to:from:subject:message-id:date:mime-version :content-type:content-transfer-encoding; s=default; bh=Jh/wPivOO lCwtQujRv1IVk+NO/Q=; b=eThC6S2kqL0xebIhO+kHaveQ8mfiwTRwGOguQ+MBd jzo9wSxF7TZ5GgAxYPmabOS8GhsYcHRGz7Tgue/dQUSdnkj5f9pAIiWdJ+p8eF2O j089aa67ldA+8jXDWuZEr9++bUxgkMBg4fwrwyUL+3EA0DJzuNBY2/PgfQEpEnx2 Ho= Received: (qmail 32488 invoked by alias); 8 Apr 2019 22:06:36 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 32340 invoked by uid 89); 8 Apr 2019 22:06:35 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-17.6 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_MANYTO, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 spammy=hang, 1216 X-HELO: mail-qt1-f179.google.com To: libc-alpha , Kwok Cheung Yeung , Joseph Myers , Andrew Stubbs , DJ Delorie , Florian Weimer From: Carlos O'Donell Subject: [PATCH] malloc: Set and reset all hooks for tracing (Bug 16573) Message-ID: <4830451c-2bd1-e019-51f3-ee3e6b806529@redhat.com> Date: Mon, 8 Apr 2019 18:06:29 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 MIME-Version: 1.0 Kwok, This is a rebase and refactor of an old patch of yours. I'm cleaning up the malloc patch queue, and this is still applicable because we haven't yet removed the old tracer for the new tracer. The exact failure you saw can't happen again today because we don't do backtraces on malloc corruption, we would have just aborted the process without a hang. However, I think the patch is still valid because _dl_addr() could do any number of operations calling back into one of the other trace handlers and hang. Does this look OK to you for master? 8< --- 8< --- 8< If an error occurs during the tracing operation, particularly during a call to lock_and_info() which calls _dl_addr, we may end up calling back into the malloc-subsystem and relock the loader lock and deadlock. For all intents and purposes the call to _dl_addr can call any of the malloc family API functions and so we should disable all tracing before calling such loader functions. This is similar to the strategy that the new malloc tracer takes when calling the real malloc, namely that all tracing ceases at the boundary to the real function and any faults at that point are the purvue of the library (though the new tracer does this on a per-thread basis in an MT-safe fashion). Since the new tracer and the hook deprecation are not yet complete we must fix these issues where we can. Tested on x86_64 with no regressions. Co-authored-by: Kwok Cheung Yeung --- ChangeLog | 15 +++++++++++ malloc/mtrace.c | 72 +++++++++++++++++++++++++++++++------------------ 2 files changed, 61 insertions(+), 26 deletions(-) diff --git a/ChangeLog b/ChangeLog index 6b7f19a1f3..786bb52400 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,18 @@ +2019-04-08 Carlos O'Donell + Kwok Cheung Yeung + + [BZ #16573] + * malloc/mtrace.c: Define prototypes for all hooks. + (set_default_hooks): New function. + (set_trace_hooks): Likewise. + (save_default_hooks): Likewise. + (tr_freehook): Use new s*_hooks functions. + (tr_mallochook): Likewise. + (tr_reallochook): Likewise. + (tr_memalignhook): Likewise. + (mtrace): Likewise. + (muntrace): Likewise. + 2019-04-08 Florian Weimer * resolv/resolv.h (RES_INSECURE1, RES_INSECURE2): Remove diff --git a/malloc/mtrace.c b/malloc/mtrace.c index a2facf65ea..7c6af9e693 100644 --- a/malloc/mtrace.c +++ b/malloc/mtrace.c @@ -121,6 +121,41 @@ lock_and_info (const void *caller, Dl_info *mem) return res; } +static void tr_freehook (__ptr_t, const __ptr_t); +static __ptr_t tr_mallochook (size_t, const __ptr_t); +static __ptr_t tr_reallochook (__ptr_t, size_t, const __ptr_t); +static __ptr_t tr_memalignhook (size_t, size_t, const __ptr_t); + +/* Set all the default non-trace hooks. */ +static __always_inline void +set_default_hooks (void) +{ + __free_hook = tr_old_free_hook; + __malloc_hook = tr_old_malloc_hook; + __realloc_hook = tr_old_realloc_hook; + __memalign_hook = tr_old_memalign_hook; +} + +/* Set all of the tracing hooks used for mtrace. */ +static __always_inline void +set_trace_hooks (void) +{ + __free_hook = tr_freehook; + __malloc_hook = tr_mallochook; + __realloc_hook = tr_reallochook; + __memalign_hook = tr_memalignhook; +} + +/* Save the current set of hooks as the default hooks. */ +static __always_inline void +save_default_hooks (void) +{ + tr_old_free_hook = __free_hook; + tr_old_malloc_hook = __malloc_hook; + tr_old_realloc_hook = __realloc_hook; + tr_old_memalign_hook = __memalign_hook; +} + static void tr_freehook (void *ptr, const void *caller) { @@ -138,12 +173,12 @@ tr_freehook (void *ptr, const void *caller) tr_break (); __libc_lock_lock (lock); } - __free_hook = tr_old_free_hook; + set_default_hooks (); if (tr_old_free_hook != NULL) (*tr_old_free_hook)(ptr, caller); else free (ptr); - __free_hook = tr_freehook; + set_trace_hooks (); __libc_lock_unlock (lock); } @@ -155,12 +190,12 @@ tr_mallochook (size_t size, const void *caller) Dl_info mem; Dl_info *info = lock_and_info (caller, &mem); - __malloc_hook = tr_old_malloc_hook; + set_default_hooks (); if (tr_old_malloc_hook != NULL) hdr = (void *) (*tr_old_malloc_hook)(size, caller); else hdr = (void *) malloc (size); - __malloc_hook = tr_mallochook; + set_trace_hooks (); tr_where (caller, info); /* We could be printing a NULL here; that's OK. */ @@ -185,16 +220,12 @@ tr_reallochook (void *ptr, size_t size, const void *caller) Dl_info mem; Dl_info *info = lock_and_info (caller, &mem); - __free_hook = tr_old_free_hook; - __malloc_hook = tr_old_malloc_hook; - __realloc_hook = tr_old_realloc_hook; + set_default_hooks (); if (tr_old_realloc_hook != NULL) hdr = (void *) (*tr_old_realloc_hook)(ptr, size, caller); else hdr = (void *) realloc (ptr, size); - __free_hook = tr_freehook; - __malloc_hook = tr_mallochook; - __realloc_hook = tr_reallochook; + set_trace_hooks (); tr_where (caller, info); if (hdr == NULL) @@ -230,14 +261,12 @@ tr_memalignhook (size_t alignment, size_t size, const void *caller) Dl_info mem; Dl_info *info = lock_and_info (caller, &mem); - __memalign_hook = tr_old_memalign_hook; - __malloc_hook = tr_old_malloc_hook; + set_default_hooks (); if (tr_old_memalign_hook != NULL) hdr = (void *) (*tr_old_memalign_hook)(alignment, size, caller); else hdr = (void *) memalign (alignment, size); - __memalign_hook = tr_memalignhook; - __malloc_hook = tr_mallochook; + set_trace_hooks (); tr_where (caller, info); /* We could be printing a NULL here; that's OK. */ @@ -305,14 +334,8 @@ mtrace (void) malloc_trace_buffer = mtb; setvbuf (mallstream, malloc_trace_buffer, _IOFBF, TRACE_BUFFER_SIZE); fprintf (mallstream, "= Start\n"); - tr_old_free_hook = __free_hook; - __free_hook = tr_freehook; - tr_old_malloc_hook = __malloc_hook; - __malloc_hook = tr_mallochook; - tr_old_realloc_hook = __realloc_hook; - __realloc_hook = tr_reallochook; - tr_old_memalign_hook = __memalign_hook; - __memalign_hook = tr_memalignhook; + save_default_hooks (); + set_trace_hooks (); #ifdef _LIBC if (!added_atexit_handler) { @@ -338,10 +361,7 @@ muntrace (void) file. */ FILE *f = mallstream; mallstream = NULL; - __free_hook = tr_old_free_hook; - __malloc_hook = tr_old_malloc_hook; - __realloc_hook = tr_old_realloc_hook; - __memalign_hook = tr_old_memalign_hook; + set_default_hooks (); fprintf (f, "= End\n"); fclose (f);