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 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6643EC433F5 for ; Wed, 13 Oct 2021 19:58:10 +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 31B9C61165 for ; Wed, 13 Oct 2021 19:58:10 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 31B9C61165 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:Content-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:From: References:Cc:To:Subject:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=cSL/iAEUkECfX2bvUDAOgkW3LOQpgyK/vFp5xI7bugQ=; b=Ll8JzxFHMPUnS9ZHN+JQw1Z85W QW5w+vA1jV2pNzLq2ttpJronsTpaMvd/HA43nBCyt7iNZPBOTDqZq4AcrLTV4fS8N3BLUW+OAQ/QV zC2AdbYhvfia6jMkDXUOXKDC6Rb1Pvq24xX3LnGW8fp6llFPJrxIhNb7hWAk/227OD6wu/ld6ZtC/ yyfvnxtr3+hvqXvKrDIClfSRQdba7ZqaSy3hBYEhSp9zy3Xqj31szcG+G2hAjwBNsaZJux6qhd/6z J3uVfyz+5vdvhNIUVEtd03Jj8brJnQlAy0S+UmcU/Ivl9EkCdHF6FDKDpbF9+50l2EiaMfvzJhz2Q IRs0N1kA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1makLv-000T8i-FC; Wed, 13 Oct 2021 19:55:51 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1makLr-000T7p-CD for linux-arm-kernel@lists.infradead.org; Wed, 13 Oct 2021 19:55:49 +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 C91701063; Wed, 13 Oct 2021 12:55:39 -0700 (PDT) Received: from [10.57.95.157] (unknown [10.57.95.157]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 47C003F70D; Wed, 13 Oct 2021 12:55:37 -0700 (PDT) Subject: Re: [PATCH 01/13] arm64: lib: __arch_clear_user(): fold fixups into body To: Mark Rutland , linux-arm-kernel@lists.infradead.org Cc: alexandru.elisei@arm.com, andrii@kernel.org, ardb@kernel.org, ast@kernel.org, broonie@kernel.org, catalin.marinas@arm.com, daniel@iogearbox.net, dvyukov@google.com, james.morse@arm.com, jean-philippe@linaro.org, jpoimboe@redhat.com, maz@kernel.org, peterz@infradead.org, suzuki.poulose@arm.com, will@kernel.org References: <20211013110059.10324-1-mark.rutland@arm.com> <20211013110059.10324-2-mark.rutland@arm.com> From: Robin Murphy Message-ID: Date: Wed, 13 Oct 2021 20:55:31 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:78.0) Gecko/20100101 Thunderbird/78.14.0 MIME-Version: 1.0 In-Reply-To: <20211013110059.10324-2-mark.rutland@arm.com> Content-Language: en-GB X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211013_125547_570160_AA326B92 X-CRM114-Status: GOOD ( 27.36 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 2021-10-13 12:00, Mark Rutland wrote: > Like other functions, __arch_clear_user() places its exception fixups in > the `.fixup` section without any clear association with > __arch_clear_user() itself. If we backtrace the fixup code, it will be > symbolized as an offset from the nearest prior symbol, which happens to > be `__entry_tramp_text_end`. Further, since the PC adjustment for the > fixup is akin to a direct branch rather than a function call, > __arch_clear_user() itself will be missing from the backtrace. > > This is confusing and hinders debugging. In general this pattern will > also be problematic for CONFIG_LIVEPATCH, since fixups often return to > their associated function, but this isn't accurately captured in the > stacktrace. > > To solve these issues for assembly functions, we must move fixups into > the body of the functions themselves, after the usual fast-path returns. > This patch does so for __arch_clear_user(). > > Inline assembly will be dealt with in subsequent patches. > > Other than the improved backtracing, there should be no functional > change as a result of this patch. Oh, I always assumed the .fixup section might have some special significance of its own. If not, all the better - modulo one possible nit below, this is fine by me. Also it's led me to see that copy_in_user() has finally left us, hooray! Guess I've got no more excuses to put off that promised usercopy rewrite other than finding the time now... > Signed-off-by: Mark Rutland > Cc: Ard Biesheuvel > Cc: Catalin Marinas > Cc: James Morse > Cc: Mark Brown > Cc: Robin Murphy > Cc: Will Deacon > --- > arch/arm64/lib/clear_user.S | 7 ++----- > 1 file changed, 2 insertions(+), 5 deletions(-) > > diff --git a/arch/arm64/lib/clear_user.S b/arch/arm64/lib/clear_user.S > index a7efb2ad2a1c..dac13df4a1ed 100644 > --- a/arch/arm64/lib/clear_user.S > +++ b/arch/arm64/lib/clear_user.S > @@ -45,13 +45,10 @@ USER(9f, sttrh wzr, [x0]) > USER(7f, sttrb wzr, [x2, #-1]) > 5: mov x0, #0 > ret > -SYM_FUNC_END(__arch_clear_user) > -EXPORT_SYMBOL(__arch_clear_user) > > - .section .fixup,"ax" The one useful purpose this did serve is to provide a handy visual cue - if you have any more concrete reason to respin the series, would you mind sticking in a little comment like "// Exception fixup" here and in the other two assembly routines, just so it's harder to overlook that the preceding ret is the normal exit path? Either way, for patches 1-3, Acked-by: Robin Murphy I got totally lost trying to follow the _ASM_EXTABLE_UACCESS_* business, but I don't think the rest of the series gets in the way of any outstanding plans either (and FWIW I always thought "fixup->fixup" was pretty awful so feel free to have an ack for patch #9 as well). Indeed, I guess the new type field should mean that we can implement "proper" address-aware fault handlers without the Itanium trick if we still need to. Cheers, Robin > - .align 2 > 7: sub x0, x2, #5 // Adjust for faulting on the final byte... > 8: add x0, x0, #4 // ...or the second word of the 4-7 byte case > 9: sub x0, x2, x0 > ret > - .previous > +SYM_FUNC_END(__arch_clear_user) > +EXPORT_SYMBOL(__arch_clear_user) > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel