From patchwork Thu Oct 31 19:20:41 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Adhemerval Zanella (Code Review)" X-Patchwork-Id: 1187647 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=sourceware.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=libc-alpha-return-106515-incoming=patchwork.ozlabs.org@sourceware.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=gnutoolchain-gerrit.osci.io Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.b="vS3eXfrq"; 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 473wDj2czWz9sNw for ; Fri, 1 Nov 2019 06:21:57 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:date:from:to:cc:message-id:subject:references :reply-to:mime-version:content-transfer-encoding:content-type; q=dns; s=default; b=QFLjUFxXadp4WZsaXiZ6IbuvpJUhjRSJaUWNwmtBA8F 0GMkysyu8qfv4J3vGS/EYXBaD9rCfJN8rv/G66SUqhXm5lSSEUFd1nfB9h/5OBaz 0jXiJe3Q9sU+1E9AuemSBBoajQBo/AppNBZfALTU+ya3RvSsE1gW1iksZbBAgZlk = 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:date:from:to:cc:message-id:subject:references :reply-to:mime-version:content-transfer-encoding:content-type; s=default; bh=iK/IcROFYUAkcF/HIV5UYd85rY4=; b=vS3eXfrqJos/UDJii hruOzUd8mDH0tFHDBBqDfa2/BlhERf+vM4o3O5H4dKI/fcxeEWO7iPaHfaddcNj4 ehvk6CAXr0cr+mllGMfbE/i6iOFsFrJY+JBUd3xeYVb51YfthNs1DGxbdGOxVzJX sprzUDjEt2D4xK5STl/gCpoFOk= Received: (qmail 120721 invoked by alias); 31 Oct 2019 19:20:55 -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 120654 invoked by uid 89); 31 Oct 2019 19:20:55 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3 autolearn=ham version=3.3.1 spammy=Blocking X-HELO: mx1.osci.io X-Gerrit-PatchSet: 1 Date: Thu, 31 Oct 2019 15:20:41 -0400 From: "Florian Weimer (Code Review)" To: libc-alpha@sourceware.org Cc: Florian Weimer Message-ID: Auto-Submitted: auto-generated X-Gerrit-MessageType: newchange Subject: [review] Block signals during the initial part of dlopen X-Gerrit-Change-Id: Iad079080ebe7442c13313ba11dc2797953faef35 X-Gerrit-Change-Number: 472 X-Gerrit-ChangeURL: X-Gerrit-Commit: 35b35a14a21a74bd82541f91c91bad7f9167a3c2 References: Reply-To: fweimer@redhat.com, fweimer@redhat.com, libc-alpha@sourceware.org MIME-Version: 1.0 Content-Disposition: inline User-Agent: Gerrit/3.0.3-75-g9005159e5d Change URL: https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/472 ...................................................................... Block signals during the initial part of dlopen Lazy binding during dlopen from a signal handler sees intermediate dynamic linker state. This has likely been always unsafe, but with the new pending NODELETE state, this is clearly incorrect. Other threads are excluded via the loader lock, but the current thread is not. Blocking signals until right before ELF constructors run is the safe thing to do. Change-Id: Iad079080ebe7442c13313ba11dc2797953faef35 Reviewed-by: Christian Brauner --- M elf/dl-open.c 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/elf/dl-open.c b/elf/dl-open.c index 9b42f2b..7322c5d 100644 --- a/elf/dl-open.c +++ b/elf/dl-open.c @@ -34,6 +34,7 @@ #include #include #include +#include #include #include @@ -52,6 +53,10 @@ /* Namespace ID. */ Lmid_t nsid; + /* Original signal mask. Used for unblocking signal handlers before + running ELF constructors. */ + sigset_t original_signal_mask; + /* Original value of _ns_global_scope_pending_adds. Set by dl_open_worker. Only valid if nsid is a real namespace (non-negative). */ @@ -510,12 +515,16 @@ if (new == NULL) { assert (mode & RTLD_NOLOAD); + __libc_signal_restore_set (&args->original_signal_mask); return; } if (__glibc_unlikely (mode & __RTLD_SPROF)) - /* This happens only if we load a DSO for 'sprof'. */ - return; + { + /* This happens only if we load a DSO for 'sprof'. */ + __libc_signal_restore_set (&args->original_signal_mask); + return; + } /* This object is directly loaded. */ ++new->l_direct_opencount; @@ -551,6 +560,7 @@ assert (_dl_debug_initialize (0, args->nsid)->r_state == RT_CONSISTENT); + __libc_signal_restore_set (&args->original_signal_mask); return; } @@ -730,6 +740,10 @@ if (mode & RTLD_GLOBAL) add_to_global_resize (new); + /* Unblock signals. Data structures are now consistent, and + application code may run. */ + __libc_signal_restore_set (&args->original_signal_mask); + /* Run the initializer functions of new objects. Temporarily disable the exception handler, so that lazy binding failures are fatal. */ @@ -819,6 +833,10 @@ args.argv = argv; args.env = env; + /* Recursive lazy binding during manipulation of the dynamic loader + structures may result in incorrect behavior. */ + __libc_signal_block_all (&args.original_signal_mask); + struct dl_exception exception; int errcode = _dl_catch_exception (&exception, dl_open_worker, &args); @@ -859,10 +877,16 @@ _dl_close_worker (args.map, true); + /* Restore the signal mask. In the success case, this + happens inside dl_open_worker. */ + __libc_signal_restore_set (&args.original_signal_mask); + /* All link_map_nodelete_pending objects should have been deleted at this point, which is why it is not necessary to reset the flag here. */ } + else + __libc_signal_restore_set (&args.original_signal_mask); assert (_dl_debug_initialize (0, args.nsid)->r_state == RT_CONSISTENT);