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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 0734FC433F5 for ; Thu, 31 Mar 2022 06:56:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc: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=W/mpabry0JDGxXI+rctYz7+lKoaofqUBG4qV90ccswM=; b=3gMe0JHT6aXWCs l44xN6gZabchH4PitbffFWQCq6xxUiA65ChO94RIR1y82I56sCuHn48VySTuZEqXglBmwbI5fdiXa YE/UZoX45zMrfLG3JCcZM7OrhzAS0Fq7+YrFdBMA/l+xUzWRx8Mb+JXVRWW599UDd5NKUN2DzpMUs ndBxw4yiW5NP9ZpRmP634vl3BoNzAwwO8idAkaLMfcTtodFWpBGTuZw3hKH3LJzpQ7oKgQFT1v8VX 9mBMmmtxwH0T2D9uSGUzSW27+riq/HW0hA7CC/rVEdZc0LHKskUkR3fDLxO9OF+ftYxukDKj2976i W+tp41nxKKijXlAH/jpQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nZoif-000xMc-6a; Thu, 31 Mar 2022 06:55:45 +0000 Received: from pandora.armlinux.org.uk ([2001:4d48:ad52:32c8:5054:ff:fe00:142]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nZoib-000xLO-MS for linux-arm-kernel@lists.infradead.org; Thu, 31 Mar 2022 06:55:43 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=armlinux.org.uk; s=pandora-2019; h=Sender:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=1P99cbgcKyenMIXAIS8yhOnAiro5GgvjDpUxTeyc0Go=; b=K++RLGxMkxvPSlyZKD7HePjIBL QcOMSQPQfXME0LgeabX+vb6UkMHq5+jQZdZyDbUVRvs631e0AM/nHYmsGz2icdNbBcwNog/1D01KL HQ9AYRV0O3lTylIPdvc6EpEiMkNKrqsQ5GFMV7y7Kk+dL+khXMgue59kDftZ39DFeaL6KwnkLEa3i cDxzA9+dc8V0g2oZEwewdDbEl3PRFt1B0oSm9+aq7iFn5DHK9XzYO4k6orRdnB6h9kLlmzkuYZY9v sghN7Stmnt53h8jb5428gpHZHGyblhqUpkA5xkg25zAoiQ+2wt75EluWY/TfAkkAiH5c4ldN3ifd2 eU6/8tDg==; Received: from shell.armlinux.org.uk ([fd8f:7570:feb6:1:5054:ff:fe00:4ec]:58036) by pandora.armlinux.org.uk with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1nZoiP-0004JW-I7; Thu, 31 Mar 2022 07:55:29 +0100 Received: from linux by shell.armlinux.org.uk with local (Exim 4.94.2) (envelope-from ) id 1nZoiK-0007On-Ax; Thu, 31 Mar 2022 07:55:24 +0100 Date: Thu, 31 Mar 2022 07:55:24 +0100 From: "Russell King (Oracle)" To: Chen Zhongjin Cc: alexander.sverdlin@nokia.com, ardb@kernel.org, linus.walleij@linaro.org, nico@fluxnic.net, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] ARM: module: Add all unwind tables when load module Message-ID: References: <20220325011252.55844-1-chenzhongjin@huawei.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20220325011252.55844-1-chenzhongjin@huawei.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220330_235541_765698_33EC6C13 X-CRM114-Status: GOOD ( 27.72 ) 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-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi, On Fri, Mar 25, 2022 at 09:12:52AM +0800, Chen Zhongjin wrote: > For EABI stack unwinding, when loading .ko module > the EXIDX sections will be added to a unwind_table list. > > However not all EXIDX sections are added because EXIDX > sections are searched by hardcoded section names. > > For functions in other sections such as .ref.text > or .kprobes.text, gcc generates seprated EXIDX sections > (such as .ARM.exidx.ref.text or .ARM.exidx.kprobes.text). > > These extra EXIDX sections are not loaded, so when unwinding > functions in these sections, we will failed with: > > unwind: Index not found xxx > > To fix that, I refactor the code for searching and adding > EXIDX sections: > > - Check section type to search EXIDX tables (0x70000001) > instead of strcmp() the hardcoded names. Then find the > corresponding text sections by their section names. > > - Add a unwind_table list in module->arch to save their own > unwind_table instead of the fixed-lenth array. > > - Save .ARM.exidx.init.text section ptr, because it should > be cleaned after module init. > > Now all EXIDX sections of .ko can be added correctly. > > Signed-off-by: Chen Zhongjin > --- > arch/arm/include/asm/module.h | 17 ++------ > arch/arm/include/asm/unwind.h | 1 + > arch/arm/kernel/module.c | 73 +++++++++++++++++------------------ > 3 files changed, 40 insertions(+), 51 deletions(-) > > diff --git a/arch/arm/include/asm/module.h b/arch/arm/include/asm/module.h > index cfffae67c04e..8139b6a33a22 100644 > --- a/arch/arm/include/asm/module.h > +++ b/arch/arm/include/asm/module.h > @@ -3,20 +3,10 @@ > #define _ASM_ARM_MODULE_H > > #include > - > -struct unwind_table; > +#include > > #ifdef CONFIG_ARM_UNWIND > -enum { > - ARM_SEC_INIT, > - ARM_SEC_DEVINIT, > - ARM_SEC_CORE, > - ARM_SEC_EXIT, > - ARM_SEC_DEVEXIT, > - ARM_SEC_HOT, > - ARM_SEC_UNLIKELY, > - ARM_SEC_MAX, > -}; > +#define ELF_SECTION_UNWIND 0x70000001 > #endif > > #define PLT_ENT_STRIDE L1_CACHE_BYTES > @@ -36,7 +26,8 @@ struct mod_plt_sec { > > struct mod_arch_specific { > #ifdef CONFIG_ARM_UNWIND > - struct unwind_table *unwind[ARM_SEC_MAX]; > + struct unwind_table unwind_list; > + struct unwind_table *init_table; > #endif > #ifdef CONFIG_ARM_MODULE_PLTS > struct mod_plt_sec core; > diff --git a/arch/arm/include/asm/unwind.h b/arch/arm/include/asm/unwind.h > index 0f8a3439902d..b51f85417f58 100644 > --- a/arch/arm/include/asm/unwind.h > +++ b/arch/arm/include/asm/unwind.h > @@ -24,6 +24,7 @@ struct unwind_idx { > > struct unwind_table { > struct list_head list; > + struct list_head mod_list; > const struct unwind_idx *start; > const struct unwind_idx *origin; > const struct unwind_idx *stop; > diff --git a/arch/arm/kernel/module.c b/arch/arm/kernel/module.c > index 549abcedf795..272f9bdeb1ed 100644 > --- a/arch/arm/kernel/module.c > +++ b/arch/arm/kernel/module.c > @@ -459,46 +459,36 @@ int module_finalize(const Elf32_Ehdr *hdr, const Elf_Shdr *sechdrs, > #ifdef CONFIG_ARM_UNWIND > const char *secstrs = (void *)hdr + sechdrs[hdr->e_shstrndx].sh_offset; > const Elf_Shdr *sechdrs_end = sechdrs + hdr->e_shnum; > - struct mod_unwind_map maps[ARM_SEC_MAX]; > - int i; > + struct unwind_table *table_list = &mod->arch.unwind_list; Please rename "table_list" to "unwind_list" so we use a consistent name for this. > > - memset(maps, 0, sizeof(maps)); > + INIT_LIST_HEAD(&table_list->mod_list); > + mod->arch.init_table = NULL; > > for (s = sechdrs; s < sechdrs_end; s++) { > - const char *secname = secstrs + s->sh_name; > + const unsigned int sectype = s->sh_type; Please loose this local variable. > > if (!(s->sh_flags & SHF_ALLOC)) > continue; > > - if (strcmp(".ARM.exidx.init.text", secname) == 0) > - maps[ARM_SEC_INIT].unw_sec = s; > - else if (strcmp(".ARM.exidx", secname) == 0) > - maps[ARM_SEC_CORE].unw_sec = s; > - else if (strcmp(".ARM.exidx.exit.text", secname) == 0) > - maps[ARM_SEC_EXIT].unw_sec = s; > - else if (strcmp(".ARM.exidx.text.unlikely", secname) == 0) > - maps[ARM_SEC_UNLIKELY].unw_sec = s; > - else if (strcmp(".ARM.exidx.text.hot", secname) == 0) > - maps[ARM_SEC_HOT].unw_sec = s; > - else if (strcmp(".init.text", secname) == 0) > - maps[ARM_SEC_INIT].txt_sec = s; > - else if (strcmp(".text", secname) == 0) > - maps[ARM_SEC_CORE].txt_sec = s; > - else if (strcmp(".exit.text", secname) == 0) > - maps[ARM_SEC_EXIT].txt_sec = s; > - else if (strcmp(".text.unlikely", secname) == 0) > - maps[ARM_SEC_UNLIKELY].txt_sec = s; > - else if (strcmp(".text.hot", secname) == 0) > - maps[ARM_SEC_HOT].txt_sec = s; > + if (sectype == ELF_SECTION_UNWIND) { I think given the amouont of indentation this causes, that: if (!(s->sh_flags & SHF_ALLOC)) || s->sh_type != ELF_SECTION_UNWIND) continue; would be preferred here. Other than that, patch looks fine, thanks. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel