From patchwork Wed Nov 27 20:30:48 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: 1201806 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-107465-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="EX/tQCqG"; 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 47NXVp6QmKz9sR6 for ; Thu, 28 Nov 2019 07:31:46 +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:subject:in-reply-to:references :reply-to:mime-version:content-transfer-encoding:content-type :message-id; q=dns; s=default; b=LDZtnqkWsUAJuQlpfHBdhoK7Vc5Zkgo i5BuZvDURv7olZ4/QS7GRhez7Uvq2C6+GreIjcm4AT5eA/T+X9/ysmnDh7zN3d4r LURbiRqLdtI3XDZpGBE4yDqCtCIq7IjzEbzNroAqaVNaebQLcnM961pImr0VlGtv iS/7xO8pS4UY= 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:subject:in-reply-to:references :reply-to:mime-version:content-transfer-encoding:content-type :message-id; s=default; bh=8PcPKqByA/wF/SGyTKCxznDJ8ZY=; b=EX/tQ CqGvUbVjGR8VDGmPljBN/+AfjMPQPDNbdd/TS/xDSsDLVWrk2cAKOwVIpC2hNpQz DkbGTIFyV8uuNgimzcYiro5/uUTiLF1xqCsfJvYF/CA/c5ZndBFWQ/hlBjiq2kwx CDalxj360ICzTuuiMmCsvq/qEkBHqKjbJ4M+Qc= Received: (qmail 115958 invoked by alias); 27 Nov 2019 20:31:02 -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 115875 invoked by uid 89); 27 Nov 2019 20:31:01 -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= X-HELO: mx1.osci.io X-Gerrit-PatchSet: 6 Date: Wed, 27 Nov 2019 15:30:48 -0500 From: "Sourceware to Gerrit sync (Code Review)" To: Florian Weimer , libc-alpha@sourceware.org Cc: Christian Brauner Auto-Submitted: auto-generated X-Gerrit-MessageType: merged Subject: [pushed] Block signals during the initial part of dlopen X-Gerrit-Change-Id: Iad079080ebe7442c13313ba11dc2797953faef35 X-Gerrit-Change-Number: 472 X-Gerrit-ChangeURL: X-Gerrit-Commit: a2e8aa0d9ea648068d8be52dd7b15f1b6a008e23 In-Reply-To: References: Reply-To: noreply@gnutoolchain-gerrit.osci.io, fweimer@redhat.com, libc-alpha@sourceware.org, christian.brauner@ubuntu.com MIME-Version: 1.0 Content-Disposition: inline User-Agent: Gerrit/3.0.3-79-g83ff7f88f1 Message-Id: <20191127203048.27C2028173@gnutoolchain-gerrit.osci.io> Sourceware to Gerrit sync has submitted this change. Change URL: https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/472 ...................................................................... Block signals during the initial part of dlopen Lazy binding in a signal handler that interrupts a dlopen 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 --- M elf/dl-open.c 1 file changed, 26 insertions(+), 2 deletions(-) Approvals: Christian Brauner: Looks good to me, approved diff --git a/elf/dl-open.c b/elf/dl-open.c index 7415c09..1051e22 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). */ @@ -524,12 +529,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; @@ -565,6 +574,7 @@ assert (_dl_debug_initialize (0, args->nsid)->r_state == RT_CONSISTENT); + __libc_signal_restore_set (&args->original_signal_mask); return; } @@ -748,6 +758,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. */ @@ -837,6 +851,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); @@ -877,10 +895,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);