From patchwork Sat May 13 20:44:42 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Masahiro Yamada X-Patchwork-Id: 13240316 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id A7483C77B7D for ; Sat, 13 May 2023 20:45:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229506AbjEMUpS (ORCPT ); Sat, 13 May 2023 16:45:18 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33766 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231196AbjEMUpR (ORCPT ); Sat, 13 May 2023 16:45:17 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7FDFC2D48; Sat, 13 May 2023 13:45:15 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 14ECB61551; Sat, 13 May 2023 20:45:15 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id DD240C4339B; Sat, 13 May 2023 20:45:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1684010714; bh=ZFbJylkRfRWvmh1oXo92Q82xN7MYbuSvRg13umh8NkQ=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=OxMJ9P2FZ0Lm3WrwMQNxNRQyoLUJUYouZU0IAlJHq3r4NZaNgxQ+AtlNqGGLIIp99 ipLObQKsMHNi51dzya3LZsECjE00veNZ2FVe6ioxKafDuaj69uIr4ZgjVkYzVmi3AR RRftpS+zk4FXgLkWXPuAcGarT9fTzykV1ahr4VfMKmW3MHaknIL9opLgAa9CU8qiE4 9cErbelnH4ZMFMXr7nq4KAeb3Lyrv1FPRdFtlalUJIxFAHgxiUI2aVWUYgTnss8qUZ FgYzRpvyTLXAMow7vaUckLUawxY3S+lqh8yiCWwcFa4NH0soyq+IfpDTMNI7wOKkBp 88KdiHnaQTAfA== From: Masahiro Yamada To: linux-kbuild@vger.kernel.org Cc: linux-kernel@vger.kernel.org, Nathan Chancellor , Nick Desaulniers , Nicolas Pitre , Nicolas Schier , Masahiro Yamada Subject: [PATCH v4 01/21] modpost: remove broken calculation of exception_table_entry size Date: Sun, 14 May 2023 05:44:42 +0900 Message-Id: <20230513204502.1593923-2-masahiroy@kernel.org> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20230513204502.1593923-1-masahiroy@kernel.org> References: <20230513204502.1593923-1-masahiroy@kernel.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-kbuild@vger.kernel.org find_extable_entry_size() is completely broken. It has awesome comments about how to calculate sizeof(struct exception_table_entry). It was based on these assumptions: - struct exception_table_entry has two fields - both of the fields have the same size Then, we came up with this equation: (offset of the second field) * 2 == (size of struct) It was true for all architectures when commit 52dc0595d540 ("modpost: handle relocations mismatch in __ex_table.") was applied. Our mathematics broke when commit 548acf19234d ("x86/mm: Expand the exception table logic to allow new handling options") introduced the third field. Now, the definition of exception_table_entry is highly arch-dependent. For x86, sizeof(struct exception_table_entry) is apparently 12, but find_extable_entry_size() sets extable_entry_size to 8. I could fix it, but I do not see much value in this code. extable_entry_size is used just for selecting a slightly different error message. If the first field ("insn") references to a non-executable section, The relocation at %s+0x%lx references section "%s" which is not executable, IOW it is not possible for the kernel to fault at that address. Something is seriously wrong and should be fixed. If the second field ("fixup") references to a non-executable section, The relocation at %s+0x%lx references section "%s" which is not executable, IOW the kernel will fault if it ever tries to jump to it. Something is seriously wrong and should be fixed. Merge the two error messages rather than adding even more complexity. Change fatal() to error() to make it continue running and catch more possible errors. Fixes: 548acf19234d ("x86/mm: Expand the exception table logic to allow new handling options") Signed-off-by: Masahiro Yamada --- scripts/mod/modpost.c | 60 +++---------------------------------------- 1 file changed, 3 insertions(+), 57 deletions(-) diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index c1c523adb139..ba4577aa4f1d 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -1292,43 +1292,6 @@ static int is_executable_section(struct elf_info* elf, unsigned int section_inde return ((elf->sechdrs[section_index].sh_flags & SHF_EXECINSTR) == SHF_EXECINSTR); } -/* - * We rely on a gross hack in section_rel[a]() calling find_extable_entry_size() - * to know the sizeof(struct exception_table_entry) for the target architecture. - */ -static unsigned int extable_entry_size = 0; -static void find_extable_entry_size(const char* const sec, const Elf_Rela* r) -{ - /* - * If we're currently checking the second relocation within __ex_table, - * that relocation offset tells us the offsetof(struct - * exception_table_entry, fixup) which is equal to sizeof(struct - * exception_table_entry) divided by two. We use that to our advantage - * since there's no portable way to get that size as every architecture - * seems to go with different sized types. Not pretty but better than - * hard-coding the size for every architecture.. - */ - if (!extable_entry_size) - extable_entry_size = r->r_offset * 2; -} - -static inline bool is_extable_fault_address(Elf_Rela *r) -{ - /* - * extable_entry_size is only discovered after we've handled the - * _second_ relocation in __ex_table, so only abort when we're not - * handling the first reloc and extable_entry_size is zero. - */ - if (r->r_offset && extable_entry_size == 0) - fatal("extable_entry size hasn't been discovered!\n"); - - return ((r->r_offset == 0) || - (r->r_offset % extable_entry_size == 0)); -} - -#define is_second_extable_reloc(Start, Cur, Sec) \ - (((Cur) == (Start) + 1) && (strcmp("__ex_table", (Sec)) == 0)) - static void report_extable_warnings(const char* modname, struct elf_info* elf, const struct sectioncheck* const mismatch, Elf_Rela* r, Elf_Sym* sym, @@ -1384,22 +1347,9 @@ static void extable_mismatch_handler(const char* modname, struct elf_info *elf, "You might get more information about where this is\n" "coming from by using scripts/check_extable.sh %s\n", fromsec, (long)r->r_offset, tosec, modname); - else if (!is_executable_section(elf, get_secindex(elf, sym))) { - if (is_extable_fault_address(r)) - fatal("The relocation at %s+0x%lx references\n" - "section \"%s\" which is not executable, IOW\n" - "it is not possible for the kernel to fault\n" - "at that address. Something is seriously wrong\n" - "and should be fixed.\n", - fromsec, (long)r->r_offset, tosec); - else - fatal("The relocation at %s+0x%lx references\n" - "section \"%s\" which is not executable, IOW\n" - "the kernel will fault if it ever tries to\n" - "jump to it. Something is seriously wrong\n" - "and should be fixed.\n", - fromsec, (long)r->r_offset, tosec); - } + else if (!is_executable_section(elf, get_secindex(elf, sym))) + error("%s+0x%lx references non-executable section '%s'\n", + fromsec, (long)r->r_offset, tosec); } static void check_section_mismatch(const char *modname, struct elf_info *elf, @@ -1574,8 +1524,6 @@ static void section_rela(const char *modname, struct elf_info *elf, /* Skip special sections */ if (is_shndx_special(sym->st_shndx)) continue; - if (is_second_extable_reloc(start, rela, fromsec)) - find_extable_entry_size(fromsec, &r); check_section_mismatch(modname, elf, &r, sym, fromsec); } } @@ -1635,8 +1583,6 @@ static void section_rel(const char *modname, struct elf_info *elf, /* Skip special sections */ if (is_shndx_special(sym->st_shndx)) continue; - if (is_second_extable_reloc(start, rel, fromsec)) - find_extable_entry_size(fromsec, &r); check_section_mismatch(modname, elf, &r, sym, fromsec); } }