From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS31976 209.132.180.0/23 X-Spam-Status: No, score=-4.0 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_EF,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED,SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 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 dcvr.yhbt.net (Postfix) with ESMTPS id F16981F462 for ; Fri, 24 May 2019 13:24:42 +0000 (UTC) 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:cc:references:from:subject:message-id:date :mime-version:in-reply-to:content-type :content-transfer-encoding; q=dns; s=default; b=WL88uPipQcht9jaa tT0bQGpAKki3Wm+AkGTUGPjFqdOeKRwd7E9lxQ5Rze3CQkNg6psY1aObTrFckszw y3XUVWp79SfoXF27GnekS6bfSDz2Ynrkj3GwOOjKci2ySs8TeXZWu98b7/Jgy8xH vuzdCprTRC4o54v8CayMjyVutYw= 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:cc:references:from:subject:message-id:date :mime-version:in-reply-to:content-type :content-transfer-encoding; s=default; bh=i11Ja3S4S3d5tr9jaOsGaM 2gzpo=; b=R1MmUwgx+NnsNaScb09COKvZAAbf3XSv8UHpeyUL9UlqztyTSUCggl /qA5hMTf77yQVlZumL2OG2ky7ErMJ171Dh3iZRdYljH9PH11wD8Ya9/flCHWHBV6 uhgr+uT5DLGcFVC1/pPC53EDbV9HI/qvit+jLBm1FFpOH0qgIKhgA= Received: (qmail 129103 invoked by alias); 24 May 2019 13:24:40 -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 Received: (qmail 128955 invoked by uid 89); 24 May 2019 13:24:26 -0000 Authentication-Results: sourceware.org; auth=none X-HELO: mail-vs1-f65.google.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=to:cc:references:from:openpgp:autocrypt:subject:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=bKah98jYLn92slTCEUUdIc372tJyoumR/MiIv8nGE64=; b=N3tR/7IgH3S7YlV92BeeKtnmt+lvE7DuVR9PKBk9CKT0vqr1FmtaN1edd2l8hPYLwo aJkyW13HMLsMewF2TgBak/MU7WjE0+9PyPunT0W9nvC7hoHOhrai4TXmWlcczfWcjZJC UBhCZnUqXwkQXejXTNuK1Megv3GGDhoyeXrkDikEQhSw9bWyM3Z+oyHvhVZUX02z3xA4 CsgC7r/s8bqlanr5bOq6ka7V+OI8HXh/JYz2qs5d5locthSgtgKbCfNEptTwCNWCDLdG n32Pt5dTEXxTpx3VnqoX5pWJw02CW0XIy6wwnFZ5icpTnsoHTKh3Q2IuXNWJ2e9BTqeV u6zA== To: Florian Weimer Cc: Carlos O'Donell , libc-alpha@sourceware.org References: <20190523133048.14922-1-adhemerval.zanella@linaro.org> <77021a69-c1a9-b41a-4396-5201915e3fa3@linaro.org> <87blztulg7.fsf@oldenburg2.str.redhat.com> <87lfyxt0fk.fsf@mid.deneb.enyo.de> <9b7c942b-ffba-4032-0d85-42a3b401d12f@linaro.org> <87o93sru1b.fsf@oldenburg2.str.redhat.com> From: Adhemerval Zanella Openpgp: preference=signencrypt Subject: Re: [PATCH] nptl: Fix deadlock on atfork handler which calls dlclose (BZ#24595) Message-ID: <0636dd42-5cdb-7bdd-5ad7-0614bad0da78@linaro.org> Date: Fri, 24 May 2019 10:24:19 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: <87o93sru1b.fsf@oldenburg2.str.redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit On 24/05/2019 08:06, Florian Weimer wrote: > * Adhemerval Zanella: > >> On 23/05/2019 16:50, Florian Weimer wrote: >>> * Adhemerval Zanella: >>> >>>> The solution sounds correct, but I don't have a strong opinion if this >>>> is really an improvement over a recursive lock plus a linked list. It >>>> potentially adds 'free' calls in fork for multithread mode if list needs >>>> to be deallocated. Also, since the locks is internal to register-atfork.c >>>> we might have a better control to make the exported interfaces not >>>> deadlock. >>> >>> Recursive locks do not prevent deadlocks if the callback launches >>> threads. I'm not sure if this can be considered a stretch. If >>> there's dlclose involved, their might also be dlopen, and threads >>> launched from an ELF constructor and so on. >> >> That's why I said the core issue is atfork handlers are really >> underspecified, and maybe it would be better if we document more >> thoughtfully what we really aim to support by atfork handlers. > > That's also true. > >>> Furthermore, I'm not sure if the implementation of __unregister_atfork >>> is correct. I think you have a potential use-after-free issue if the >>> callback deregisters a fork handler. > >> The possible way I can think of is if an atfork handler from a module >> try to dlclose itself and I am not sure how valid it is. > > dlclose'ing itself is not really valid, I think. It can only work if > the dlclose call is a tail call. I would work towards not make dlclose itself valid, do you see any reason to allow to do so? > > __unregister_atfork still adds a use-after-free issue, even with the > copy-on-write list: If a dlclose unregisters a fork handler, we will > still attempt to call it subsequently if it was on the list before. No > data race is involved if the dlclose call happens itself in a fork > handler (so we are back to the territory of the bug). > > So the simple reference counting scheme I proposed is probably not the > right solution, and we need a different data structure My understanding is if the idea of dlclose on atfork handler is to not just unregister the library but also prevent further arfork handler to been called (since __cxa_finalize will call __unregister_atfork), I still think that the recursive mutex plus double-linked list is the best a suitable solution if we assume two premisses: - It is invalid to dlclose itself. - Atfork handlers should be local to modules. Both should avoid imho the use-after-free issue since the callback won't try to deregister an already used entry in atfork handler list walking.