From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.3 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 59210C433DF for ; Mon, 18 May 2020 15:59:49 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 09AFE20758 for ; Mon, 18 May 2020 15:59:49 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="G8z4r28V" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 09AFE20758 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=4QH+QoDq+jchTAnGwFWXIjnx6sR3gfrlnaES5kOGGSw=; b=G8z4r28VZWaV13 NLckI+CMuS64/YePmYPOSDuwZIXRVjEqzA/fweiGhQUckiFxxeGH5Dj2mrh6BHSzG/nceeqDupHv9 +lctT1RUhCMSIVE3jrZ7PsC5SmOhN/Csqd3/djfmMOmobjFAA+d7HDcV1T0TsM7iVupqhzLM5sGc6 e26RSzECloelih86pbF9gvOWjbM3TTVGU1FMlMJ3ll7sWASPtsswD3ex/D5mvQWPUPzVuj2p/+Z/3 YOE97QUhBcU8v7SfIQs0gXHhtHgLluOayBC2hAknOp0/3+iY+4+IhWYElHG2WK1XuYcgDtYj7wlHt vhURxx26WKaEry1G8ACw==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jaiB1-000207-Cw; Mon, 18 May 2020 15:59:39 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jaiAy-0001zW-CW for linux-arm-kernel@lists.infradead.org; Mon, 18 May 2020 15:59:38 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 90156106F; Mon, 18 May 2020 08:59:30 -0700 (PDT) Received: from arm.com (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id A26303F52E; Mon, 18 May 2020 08:59:29 -0700 (PDT) Date: Mon, 18 May 2020 16:59:27 +0100 From: Dave Martin To: dankis01 Subject: Re: [PATCH] arm64: vdso: Fix CFI info in sigreturn. Message-ID: <20200518155926.GA21779@arm.com> References: <20200515162020.78169-1-daniel.kiss@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200515162020.78169-1-daniel.kiss@arm.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200518_085936_470109_C17346AF X-CRM114-Status: GOOD ( 22.26 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: mark.rutland@arm.com, Tamas Zsoldos , Vincenzo.Frascino@arm.com, linux-arm-kernel@lists.infradead.org, Daniel Kiss Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, May 15, 2020 at 06:20:20PM +0200, dankis01 wrote: > When the signal handler is called the registers set up as the return address > points to the __kernel_rt_sigreturn. The NOP here is the placeholder of the > branch and link instruction that "calls" the signal handler. In case of a > return address the unwinder identifies the location of the caller because > in some cases the return address might not exist. Since the .cfi_startproc > is after the NOP, it won't be associated with any location and the > unwinder will stop walking. > > This change corrects the generated EHFrames only. This is a can of worms. Which unwinder are you look at, and what do other unwinders do? Are you sure the unwinder is doing something valid? Is this a newly observed problem, or has it happened forever? Why should there be any instruction that "calls" the signal handler? In the case is a SIGSEGV the affected instruction is after the pc and not before it; for an asynchronous signal and notion of a "calling" instruction is nonsense. Certainly I've seen correct unwinding through signal handlers with glibc and gdb, but I hadn't tried everything... > > Signed-off-by: Daniel Kiss > Signed-off-by: Tamas Zsoldos > --- > arch/arm64/kernel/vdso/sigreturn.S | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/kernel/vdso/sigreturn.S b/arch/arm64/kernel/vdso/sigreturn.S > index 12324863d5c2..5d50ee92faa4 100644 > --- a/arch/arm64/kernel/vdso/sigreturn.S > +++ b/arch/arm64/kernel/vdso/sigreturn.S > @@ -13,13 +13,13 @@ > > .text > > - nop > -SYM_FUNC_START(__kernel_rt_sigreturn) > .cfi_startproc > .cfi_signal_frame > .cfi_def_cfa x29, 0 > .cfi_offset x29, 0 * 8 > .cfi_offset x30, 1 * 8 Hmm, recovering x29,x30 like this will be wrong if the signal handler munges sigcontext in the meantime (say, doing some kind of userspace context switch). They should be pulled out of sigcontext instead really. AFAIK, that's what ".cfi_signal_frame" is supposed to tell the unwinder. I'm not sure why we have these additional, conflicting annotations here. Any ideas, Will? This probably isn't related to the bug here, but it would be good to understand. > + nop /* placeholder for bl signalhandler */ Will can correct me on this, but I seem to remember something about nop being there for padding, so that there is a guaranteed gap between unwind entries. I can't remember the precise reasoning, but there are some nasty edge cases connected with the fact that the linker can place another random unwind block from another .o immediately before this one. Cheers ---Dave > +SYM_FUNC_START(__kernel_rt_sigreturn) > mov x8, #__NR_rt_sigreturn > svc #0 > .cfi_endproc > -- > 2.17.1 [...] Cheers ---Dave _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel