From patchwork Fri Nov 15 16:02:04 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: 1195717 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-107120-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="kkCm3Igm"; 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 47F35f5zx5z9s4Y for ; Sat, 16 Nov 2019 03:02:30 +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:subject:in-reply-to:references :reply-to:mime-version:content-transfer-encoding:content-type :message-id; q=dns; s=default; b=NdDCQvSkZMwO5m1C9mVR/VEkdee/9/u QtcG47wBIuOaXQUYGkIfB9GuiF6M0BKftGheo1iKFR45YYHRsaRXLJsyeF0vnr9Y 5KeZbn5zi1Rif9q4HhyL0gDD5DWBSGRRLMO1bHOavONlBS7PBrq+JhTpduVYCV31 NYICyJhlqepQ= 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:subject:in-reply-to:references :reply-to:mime-version:content-transfer-encoding:content-type :message-id; s=default; bh=i9XcWowBjaCL/VvfyG43ep9q5h0=; b=kkCm3 IgmjeCH1nTzxjNq0ktTxlU6SuXmZA8W9IGDEdD7b9HcIYj1Tsqm/kkxxEwtGLOMp YmAw3PXAGYVeFnhcEqbeWFoYAhrH/T/ndZup5A0UbHK4Rvh0yRUSqRjrV0WFdlzc 3Lxu6A4pMuEPMY/a//Q9dS88E89WjGtRpUHgsM= Received: (qmail 7932 invoked by alias); 15 Nov 2019 16:02:19 -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 7870 invoked by uid 89); 15 Nov 2019 16:02:19 -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: 3 Date: Fri, 15 Nov 2019 11:02:04 -0500 From: "Florian Weimer (Code Review)" To: Florian Weimer , Christian Brauner , libc-alpha@sourceware.org Auto-Submitted: auto-generated X-Gerrit-MessageType: newpatchset Subject: [review v3] Block signals during the initial part of dlopen X-Gerrit-Change-Id: Iad079080ebe7442c13313ba11dc2797953faef35 X-Gerrit-Change-Number: 472 X-Gerrit-ChangeURL: X-Gerrit-Commit: e11ab194f3a20809c35b5d3a7e439131079877ba In-Reply-To: References: Reply-To: fweimer@redhat.com, fweimer@redhat.com, libc-alpha@sourceware.org, christian.brauner@ubuntu.com MIME-Version: 1.0 Content-Disposition: inline User-Agent: Gerrit/3.0.3-76-gf8b6da0ab5 Message-Id: <20191115160205.C2C2828172@gnutoolchain-gerrit.osci.io> 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(-) diff --git a/elf/dl-open.c b/elf/dl-open.c index db870ed..b9b4319 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; } @@ -733,6 +743,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. */ @@ -822,6 +836,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); @@ -862,10 +880,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);