public inbox for linux-kbuild@vger.kernel.org
 help / color / mirror / Atom feed
From: Masahiro Yamada <masahiroy@kernel.org>
To: linux-kbuild@vger.kernel.org
Cc: linux-kernel@vger.kernel.org,
	Nathan Chancellor <nathan@kernel.org>,
	Nick Desaulniers <ndesaulniers@google.com>,
	Nicolas Pitre <npitre@baylibre.com>,
	Nicolas Schier <nicolas@fjasle.eu>,
	Masahiro Yamada <masahiroy@kernel.org>
Subject: [PATCH v4 01/21] modpost: remove broken calculation of exception_table_entry size
Date: Sun, 14 May 2023 05:44:42 +0900	[thread overview]
Message-ID: <20230513204502.1593923-2-masahiroy@kernel.org> (raw)
In-Reply-To: <20230513204502.1593923-1-masahiroy@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 <masahiroy@kernel.org>
---

 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);
 	}
 }
-- 
2.39.2


  reply	other threads:[~2023-05-13 20:45 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-13 20:44 [PATCH v4 00/21] Unify <linux/export.h> and <asm/export.h>, remove EXPORT_DATA_SYMBOL(), faster TRIM_UNUSED_KSYMS Masahiro Yamada
2023-05-13 20:44 ` Masahiro Yamada [this message]
2023-05-13 20:44 ` [PATCH v4 02/21] modpost: remove fromsym info in __ex_table section mismatch warning Masahiro Yamada
2023-05-13 20:44 ` [PATCH v4 03/21] modpost: remove get_prettyname() Masahiro Yamada
2023-05-13 20:44 ` [PATCH v4 04/21] modpost: squash report_extable_warnings() into extable_mismatch_handler() Masahiro Yamada
2023-05-13 20:44 ` [PATCH v4 05/21] modpost: squash report_sec_mismatch() into default_mismatch_handler() Masahiro Yamada
2023-05-13 20:44 ` [PATCH v4 06/21] modpost: clean up is_executable_section() Masahiro Yamada
2023-05-13 20:44 ` [PATCH v4 07/21] modpost: squash extable_mismatch_handler() into default_mismatch_handler() Masahiro Yamada
2023-05-13 20:44 ` [PATCH v4 08/21] modpost: pass 'tosec' down to default_mismatch_handler() Masahiro Yamada
2023-05-13 20:44 ` [PATCH v4 09/21] modpost: pass section index to find_elf_symbol2() Masahiro Yamada
2023-05-13 20:44 ` [PATCH v4 10/21] modpost: simplify find_elf_symbol() Masahiro Yamada
2023-05-13 20:44 ` [PATCH v4 11/21] modpost: rename find_elf_symbol() and find_elf_symbol2() Masahiro Yamada
2023-05-13 20:44 ` [PATCH v4 12/21] modpost: unify 'sym' and 'to' in default_mismatch_handler() Masahiro Yamada
2023-05-13 20:44 ` [PATCH v4 13/21] modpost: replace r->r_offset, r->r_addend with faddr, taddr Masahiro Yamada
2023-05-13 20:44 ` [PATCH v4 14/21] modpost: remove is_shndx_special() check from section_rel(a) Masahiro Yamada
2023-05-13 20:44 ` [PATCH v4 15/21] modpost: pass struct module pointer to check_section_mismatch() Masahiro Yamada
2023-05-13 20:44 ` [PATCH v4 16/21] kbuild: generate KSYMTAB entries by modpost Masahiro Yamada
2023-05-13 20:44 ` [PATCH v4 17/21] ia64,export.h: replace EXPORT_DATA_SYMBOL* with EXPORT_SYMBOL* Masahiro Yamada
2023-05-13 20:44 ` [PATCH v4 18/21] modpost: check static EXPORT_SYMBOL* by modpost again Masahiro Yamada
2023-05-13 20:45 ` [PATCH v4 19/21] modpost: squash sym_update_namespace() into sym_add_exported() Masahiro Yamada
2023-05-13 20:45 ` [PATCH v4 20/21] modpost: use null string instead of NULL pointer for default namespace Masahiro Yamada
2023-05-13 20:45 ` [PATCH v4 21/21] kbuild: implement CONFIG_TRIM_UNUSED_KSYMS without recursion Masahiro Yamada
2023-05-14 12:43 ` [PATCH v4 00/21] Unify <linux/export.h> and <asm/export.h>, remove EXPORT_DATA_SYMBOL(), faster TRIM_UNUSED_KSYMS Masahiro Yamada

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230513204502.1593923-2-masahiroy@kernel.org \
    --to=masahiroy@kernel.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nathan@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=nicolas@fjasle.eu \
    --cc=npitre@baylibre.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox