From owner-freebsd-current@freebsd.org Mon Oct 23 09:49:01 2017 Return-Path: Delivered-To: freebsd-current@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 268E9E473E0 for ; Mon, 23 Oct 2017 09:49:01 +0000 (UTC) (envelope-from tijl@freebsd.org) Received: from mailrelay116.isp.belgacom.be (mailrelay116.isp.belgacom.be [195.238.20.143]) (using TLSv1.2 with cipher RC4-SHA (128/128 bits)) (Client CN "relay.skynet.be", Issuer "GlobalSign Organization Validation CA - SHA256 - G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 51E186A71B; Mon, 23 Oct 2017 09:48:59 +0000 (UTC) (envelope-from tijl@freebsd.org) X-Belgacom-Dynamic: yes IronPort-PHdr: =?us-ascii?q?9a23=3AgTcnJR+wFUIFhP9uRHKM819IXTAuvvDOBiVQ1KB4?= =?us-ascii?q?2uwcTK2v8tzYMVDF4r011RmSDNWds6oMotGVmpioYXYH75eFvSJKW713fDhBt/?= =?us-ascii?q?8rmRc9CtWOE0zxIa2iRSU7GMNfSA0tpCnjYgBaF8nkelLdvGC54yIMFRXjLwp1?= =?us-ascii?q?Ifn+FpLPg8it2e2//57ebx9UiDahfLh/MAi4oQLNu8cMnIBsMLwxyhzHontJf+?= =?us-ascii?q?RZ22ZlLk+Nkhj/+8m94odt/zxftPw9+cFAV776f7kjQrxDEDsmKWE169b1uhTF?= =?us-ascii?q?UACC+2ETUmQSkhpPHgjF8BT3VYr/vyfmquZw3jSRMNboRr4oRzut86ZrSAfpiC?= =?us-ascii?q?gZMT457HrXgdF0gK5CvR6tuwBzz4vSbYqINvRxY7ndcMsUS2RBQMhfSi9PDYGy?= =?us-ascii?q?b4QAE+UPMv1Vr5X/qlcSsReyGRWgCP3pxzRVhnH2x6o60+E5HA/Y3QwgHdYOu2?= =?us-ascii?q?nKodXyM6cSSv66zKzSwjXFdPNdxDDw6JTJchAjpfGBRrJ+cdDPxkYxCg7Lk1ue?= =?us-ascii?q?pZbiPzOP2eQAqm6W5PduW+Kojm4osQBxoj63y8cjjInJm5gZylfe9SV2xos+ON?= =?us-ascii?q?62SFZjbNOnDJdcrT+WO5drTs84TGxltjw2x74HtJO9YSME0o4oxwTFZPyCa4WI?= =?us-ascii?q?5xXjW/uPLjpgn3Jlfa6/hw618Ui91u3wTsm030hOripCitTMtWoC1xjS6siCVP?= =?us-ascii?q?R95ECh1SyT1wDS6OFEJVo4mrbcK54m2b4/iJ8Tvl7FHi/tgkn2i7WWdko89uip?= =?us-ascii?q?7eTofKnmq4efOoNokA3yLLgiltGlDek3PAUCRWuW9OWk2L3m50L5QbFKjvMskq?= =?us-ascii?q?netZDXPdgbpq+7Aw9RyYsj5Qy/ACm439sDhnkIMUhJeBWdj4jmI13OOuz3De+j?= =?us-ascii?q?g1Swlzdm3/PGPrr6ApXRL3jDk6zucqh560NHxwozyMpQ55NQCr0bPP3zXUrx55?= =?us-ascii?q?TkCUoQNQCuzu/8QOl63IcTQyrbC6mVKq7bqkOgyPgiLsO3SMkSojmreNY/4Pu7?= =?us-ascii?q?sX47nRc2eq6y0J4ebmvwSuhnIUGxT2Dhj/06PSENpAVoH7+is0GLTTMGPyX6ZK?= =?us-ascii?q?k7/DxuTdv+VYo=3D?= X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: =?us-ascii?q?A2DIAwCXue1Z/1YryVBcGgEBAQECAQEBA?= =?us-ascii?q?QgBAQEBFQEBAQECAQEBAQgBAQEBgzRUEG4UE48NjkwBAYF5MQFJh1CPfy2FGAK?= =?us-ascii?q?ESEMVAQEBAQEBAQEBAQFqKII4JAGCQQEFJxMcIxALEgYJJQ8SGBAOBhMUB4ltA?= =?us-ascii?q?xkMrVE6hzcNg1kBAQEBAQEBAwEBAQEBAR0Fgy6FQIJ1NYJeVIFThXUFoSo8h2S?= =?us-ascii?q?IF4RsgQCSLkiMRIopNSKBW1UyCIMtglwcgWk+NgGJNSqCGgEBAQ?= X-IPAS-Result: =?us-ascii?q?A2DIAwCXue1Z/1YryVBcGgEBAQECAQEBAQgBAQEBFQEBAQE?= =?us-ascii?q?CAQEBAQgBAQEBgzRUEG4UE48NjkwBAYF5MQFJh1CPfy2FGAKESEMVAQEBAQEBA?= =?us-ascii?q?QEBAQFqKII4JAGCQQEFJxMcIxALEgYJJQ8SGBAOBhMUB4ltAxkMrVE6hzcNg1k?= =?us-ascii?q?BAQEBAQEBAwEBAQEBAR0Fgy6FQIJ1NYJeVIFThXUFoSo8h2SIF4RsgQCSLkiMR?= =?us-ascii?q?IopNSKBW1UyCIMtglwcgWk+NgGJNSqCGgEBAQ?= Received: from 86.43-201-80.adsl-dyn.isp.belgacom.be (HELO kalimero.tijl.coosemans.org) ([80.201.43.86]) by relay.skynet.be with ESMTP; 23 Oct 2017 11:47:46 +0200 Received: from kalimero.tijl.coosemans.org (kalimero.tijl.coosemans.org [127.0.0.1]) by kalimero.tijl.coosemans.org (8.15.2/8.15.2) with ESMTP id v9N9lk8e074903; Mon, 23 Oct 2017 11:47:46 +0200 (CEST) (envelope-from tijl@FreeBSD.org) Date: Mon, 23 Oct 2017 11:47:46 +0200 From: Tijl Coosemans To: Andreas Tobler Cc: Konstantin Belousov , freebsd-current@FreeBSD.org, gerald@FreeBSD.org Subject: Re: Segfault in _Unwind_* code called from pthread_exit Message-ID: <20171023114746.054037b8@kalimero.tijl.coosemans.org> In-Reply-To: References: <20170823163707.096f93ab@kalimero.tijl.coosemans.org> <20170824154235.GD1700@kib.kiev.ua> <20170824180830.199885b0@kalimero.tijl.coosemans.org> <20170825173851.09116ddc@kalimero.tijl.coosemans.org> <20170825234442.GO1700@kib.kiev.ua> <20170826202813.1240a1ef@kalimero.tijl.coosemans.org> <20170826184034.GR1700@kib.kiev.ua> <20171022021835.07ffd30e@kalimero.tijl.coosemans.org> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 23 Oct 2017 09:49:01 -0000 On Sun, 22 Oct 2017 23:05:15 +0200 Andreas Tobler wrote: > On 22.10.17 02:18, Tijl Coosemans wrote: >> On Sat, 21 Oct 2017 22:02:38 +0200 Andreas Tobler wrote: >>> On 26.08.17 20:40, Konstantin Belousov wrote: >>>> On Sat, Aug 26, 2017 at 08:28:13PM +0200, Tijl Coosemans wrote: >>>>> On Sat, 26 Aug 2017 02:44:42 +0300 Konstantin Belousov wote: >>>>>> How does llvm unwinder detects that the return address is a garbage ? >>>>> >>>>> It just stops unwinding when it can't find frame information (stored in >>>>> .eh_frame sections). GCC unwinder doesn't give up yet and checks if the >>>>> return address points to the signal trampoline (which means the current >>>>> frame is that of a signal handler). It has built-in knowledge of how to >>>>> unwind to the signal trampoline frame. >>>> So llvm just gives up on signal frames ? >>>> >>>>> A noreturn attribute isn't enough. You can still unwind such functions. >>>>> They are allowed to throw exceptions for example. >>>> Ok. >>>> >>>>> I did consider using >>>>> a CFI directive (see patch below) and it works, but it's architecture >>>>> specific and it's inserted after the function prologue so there's still >>>>> a window of a few instructions where a stack unwinder will try to use >>>>> the return address. >>>>> >>>>> Index: lib/libthr/thread/thr_create.c >>>>> =================================================================== >>>>> --- lib/libthr/thread/thr_create.c (revision 322802) >>>>> +++ lib/libthr/thread/thr_create.c (working copy) >>>>> @@ -251,6 +251,7 @@ create_stack(struct pthread_attr *pattr) >>>>> static void >>>>> thread_start(struct pthread *curthread) >>>>> { >>>>> + __asm(".cfi_undefined %rip"); >>>>> sigset_t set; >>>>> >>>>> if (curthread->attr.suspend == THR_CREATE_SUSPENDED) >>>> >>>> I like this approach much more than the previous patch. What can be >>>> done is to provide asm trampoline which calls thread_start(). There you >>>> can add the .cfi_undefined right at the entry. >>>> >>>> It is somewhat more work than just setting the return address on the >>>> kernel-constructed pseudo stack frame, but I believe this is ultimately >>>> correct way. You still can do it only on some arches, if you do not >>>> have incentive to code asm for all of them. >>>> >>>> Also crt1 probably should get the same treatment, despite we already set >>>> %rbp to zero AFAIR. >>> >>> Did some commit result out of this discussion or is this subject still >>> under investigation? >>> >>> Curious because I got this gcc PR: >>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82635 > > If I add the above to lib/libthr/thread/thr_create.c the mentioned PR works. > >> Sorry, but I didn't and won't have time to work on this. > > Np. > >> Ideally I think there should be a function attribute to mark functions >> as entry points. The compiler would add ".cfi_undefined %rip" to such >> functions (and maybe optimise the function prologue because there are >> no caller registers that need to be preserved). If you have connections >> in the GCC community maybe you could discuss that with them. > > Well, from my understanding I'd have to teach every compiler to do so, > right? (Beside that I do not know how to.) > > I think we need another solution to find out if an unwind context is > garbage. > > I'll take a look at how llvm does this w/o segfaulting. Both LLVM and GCC libgcc_s lookup the return address in the CFI tables to see which function it belongs to. Both fail to find the address in the table. LLVM then stops unwinding. GCC looks at the memory to see if the instructions are those of the signal trampoline and segfaults if the memory is unreadable. The return address of the entry point can be anything including addresses that do appear in the CFI table, because thread stacks can be supplied by the user (pthread_attr_setstack(3)). So it's not enough to avoid the segfault. Either the kernel has to set the return address to NULL like my first patch or the entry point has to be annotated with .cfi_undefined %rip to mark the return address invalid.