[PATCH 0/7] Detect future mis-uses of __ex_table section.

[PATCH 0/7] Detect future mis-uses of __ex_table section.

[Quentin Casasnovas]

Hi,

This patch-set adds some sanity checks on the __ex_table section so that it
makes it much harder to introduce wrong entries in there.  It works by
checking every relocation in __ex_table and making sure it points to an
executable section and does not point to a list of black-listed sections,
like .altinstr_replacement.  Patch 6 adds a new script, check_extable.sh,
so it can be used to get more human readable diagnostics on where the wrong
ex_table entry was added in the code.  Here's a truncated example output
from v4.0-rc2:

 Error: found a reference to .altinstr_replacement in __ex_table:
 	xrstor_state at /home/quentin/linux-2.6/./arch/x86/include/asm/xsave.h:179
 	 (inlined by) fpu_xrstor_checking at /home/quentin/linux-2.6/./arch/x86/include/asm/xsave.h:207
 	 (inlined by) fpu_restore_checking at /home/quentin/linux-2.6/./arch/x86/include/asm/fpu-internal.h:284
 	 (inlined by) kvm_load_guest_fpu at /home/quentin/linux-2.6/arch/x86/kvm/x86.c:6986

This is to prevent future misuses of the __ex_table entry like there was
for xsaves/xrstors for example, see upstream commit 06e5801b
("x86/fpu/xsaves: Fix improper uses of __ex_table").

This will potentially break the build for other architectures (tested on
x86-64 and sparc), since there might be exotic sections in there that I
don't know about.

Quentin

[PATCH 1/7] modpost: add strict white-listing when referencing sections.

[Quentin Casasnovas]

Prints a warning when a section references a section outside a strict
white-list.  This will be useful to print a warning if __ex_table
references a non-executable section.

Signed-off-by: Quentin Casasnovas <quentin.casasnovas@oracle.com>
---
 scripts/mod/modpost.c | 34 +++++++++++++++++++---------------
 1 file changed, 19 insertions(+), 15 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index d439856..7094a57 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -925,7 +925,8 @@ enum mismatch {
 
 struct sectioncheck {
 	const char *fromsec[20];
-	const char *tosec[20];
+	const char *bad_tosec[20];
+	const char *good_tosec[20];
 	enum mismatch mismatch;
 	const char *symbol_white_list[20];
 };
@@ -936,19 +937,19 @@ static const struct sectioncheck sectioncheck[] = {
  */
 {
 	.fromsec = { TEXT_SECTIONS, NULL },
-	.tosec   = { ALL_INIT_SECTIONS, NULL },
+	.bad_tosec = { ALL_INIT_SECTIONS, NULL },
 	.mismatch = TEXT_TO_ANY_INIT,
 	.symbol_white_list = { DEFAULT_SYMBOL_WHITE_LIST, NULL },
 },
 {
 	.fromsec = { DATA_SECTIONS, NULL },
-	.tosec   = { ALL_XXXINIT_SECTIONS, NULL },
+	.bad_tosec = { ALL_XXXINIT_SECTIONS, NULL },
 	.mismatch = DATA_TO_ANY_INIT,
 	.symbol_white_list = { DEFAULT_SYMBOL_WHITE_LIST, NULL },
 },
 {
 	.fromsec = { DATA_SECTIONS, NULL },
-	.tosec   = { INIT_SECTIONS, NULL },
+	.bad_tosec = { INIT_SECTIONS, NULL },
 	.mismatch = DATA_TO_ANY_INIT,
 	.symbol_white_list = {
 		"*_template", "*_timer", "*_sht", "*_ops",
@@ -957,54 +958,54 @@ static const struct sectioncheck sectioncheck[] = {
 },
 {
 	.fromsec = { TEXT_SECTIONS, NULL },
-	.tosec   = { ALL_EXIT_SECTIONS, NULL },
+	.bad_tosec = { ALL_EXIT_SECTIONS, NULL },
 	.mismatch = TEXT_TO_ANY_EXIT,
 	.symbol_white_list = { DEFAULT_SYMBOL_WHITE_LIST, NULL },
 },
 {
 	.fromsec = { DATA_SECTIONS, NULL },
-	.tosec   = { ALL_EXIT_SECTIONS, NULL },
+	.bad_tosec = { ALL_EXIT_SECTIONS, NULL },
 	.mismatch = DATA_TO_ANY_EXIT,
 	.symbol_white_list = { DEFAULT_SYMBOL_WHITE_LIST, NULL },
 },
 /* Do not reference init code/data from meminit code/data */
 {
 	.fromsec = { ALL_XXXINIT_SECTIONS, NULL },
-	.tosec   = { INIT_SECTIONS, NULL },
+	.bad_tosec = { INIT_SECTIONS, NULL },
 	.mismatch = XXXINIT_TO_SOME_INIT,
 	.symbol_white_list = { DEFAULT_SYMBOL_WHITE_LIST, NULL },
 },
 /* Do not reference exit code/data from memexit code/data */
 {
 	.fromsec = { ALL_XXXEXIT_SECTIONS, NULL },
-	.tosec   = { EXIT_SECTIONS, NULL },
+	.bad_tosec = { EXIT_SECTIONS, NULL },
 	.mismatch = XXXEXIT_TO_SOME_EXIT,
 	.symbol_white_list = { DEFAULT_SYMBOL_WHITE_LIST, NULL },
 },
 /* Do not use exit code/data from init code */
 {
 	.fromsec = { ALL_INIT_SECTIONS, NULL },
-	.tosec   = { ALL_EXIT_SECTIONS, NULL },
+	.bad_tosec = { ALL_EXIT_SECTIONS, NULL },
 	.mismatch = ANY_INIT_TO_ANY_EXIT,
 	.symbol_white_list = { DEFAULT_SYMBOL_WHITE_LIST, NULL },
 },
 /* Do not use init code/data from exit code */
 {
 	.fromsec = { ALL_EXIT_SECTIONS, NULL },
-	.tosec   = { ALL_INIT_SECTIONS, NULL },
+	.bad_tosec = { ALL_INIT_SECTIONS, NULL },
 	.mismatch = ANY_EXIT_TO_ANY_INIT,
 	.symbol_white_list = { DEFAULT_SYMBOL_WHITE_LIST, NULL },
 },
 {
 	.fromsec = { ALL_PCI_INIT_SECTIONS, NULL },
-	.tosec   = { INIT_SECTIONS, NULL },
+	.bad_tosec = { INIT_SECTIONS, NULL },
 	.mismatch = ANY_INIT_TO_ANY_EXIT,
 	.symbol_white_list = { NULL },
 },
 /* Do not export init/exit functions or data */
 {
 	.fromsec = { "__ksymtab*", NULL },
-	.tosec   = { INIT_SECTIONS, EXIT_SECTIONS, NULL },
+	.bad_tosec = { INIT_SECTIONS, EXIT_SECTIONS, NULL },
 	.mismatch = EXPORT_TO_INIT_EXIT,
 	.symbol_white_list = { DEFAULT_SYMBOL_WHITE_LIST, NULL },
 }
@@ -1018,9 +1019,12 @@ static const struct sectioncheck *section_mismatch(
 	const struct sectioncheck *check = &sectioncheck[0];
 
 	for (i = 0; i < elems; i++) {
-		if (match(fromsec, check->fromsec) &&
-		    match(tosec, check->tosec))
-			return check;
+		if (match(fromsec, check->fromsec)) {
+			if (check->bad_tosec[0] && match(tosec, check->bad_tosec))
+				return check;
+			if (check->good_tosec[0] && !match(tosec, check->good_tosec))
+				return check;
+		}
 		check++;
 	}
 	return NULL;
-- 
2.0.5

[PATCH 2/7] modpost: add .sched.text and .kprobes.text to the TEXT_SECTIONS list.

[Quentin Casasnovas]

sched.text and .kprobes.text should behave exactly like .text with regards
to how we should warn about referencing sections which might get discarded
at runtime.

Signed-off-by: Quentin Casasnovas <quentin.casasnovas@oracle.com>
---
 scripts/mod/modpost.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 7094a57..8cef46b 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -873,7 +873,8 @@ static void check_section(const char *modname, struct elf_info *elf,
 #define ALL_EXIT_SECTIONS EXIT_SECTIONS, ALL_XXXEXIT_SECTIONS
 
 #define DATA_SECTIONS ".data", ".data.rel"
-#define TEXT_SECTIONS ".text", ".text.unlikely"
+#define TEXT_SECTIONS ".text", ".text.unlikely", ".sched.text", \
+		".kprobes.text"
 
 #define INIT_SECTIONS      ".init.*"
 #define MEM_INIT_SECTIONS  ".meminit.*"
-- 
2.0.5

[PATCH 3/7] modpost: add handler function pointer to sectioncheck.

[Quentin Casasnovas]

This will be useful when we want to have special handlers which need to go
through more hops to print useful information to the user.

Signed-off-by: Quentin Casasnovas <quentin.casasnovas@oracle.com>
---
 scripts/mod/modpost.c | 68 +++++++++++++++++++++++++++++++--------------------
 1 file changed, 42 insertions(+), 26 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 8cef46b..0f48f8b 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -930,6 +930,10 @@ struct sectioncheck {
 	const char *good_tosec[20];
 	enum mismatch mismatch;
 	const char *symbol_white_list[20];
+	void (*handler)(const char *modname, struct elf_info *elf,
+			const struct sectioncheck* const mismatch,
+			Elf_Rela *r, Elf_Sym *sym, const char *fromsec);
+
 };
 
 static const struct sectioncheck sectioncheck[] = {
@@ -1417,37 +1421,49 @@ static void report_sec_mismatch(const char *modname,
 	fprintf(stderr, "\n");
 }
 
-static void check_section_mismatch(const char *modname, struct elf_info *elf,
-				   Elf_Rela *r, Elf_Sym *sym, const char *fromsec)
+static void default_mismatch_handler(const char *modname, struct elf_info *elf,
+				     const struct sectioncheck* const mismatch,
+				     Elf_Rela *r, Elf_Sym *sym, const char *fromsec)
 {
 	const char *tosec;
-	const struct sectioncheck *mismatch;
+	Elf_Sym *to;
+	Elf_Sym *from;
+	const char *tosym;
+	const char *fromsym;
 
 	tosec = sec_name(elf, get_secindex(elf, sym));
-	mismatch = section_mismatch(fromsec, tosec);
+	from = find_elf_symbol2(elf, r->r_offset, fromsec);
+	fromsym = sym_name(elf, from);
+	to = find_elf_symbol(elf, r->r_addend, sym);
+	tosym = sym_name(elf, to);
+
+	if (!strncmp(fromsym, "reference___initcall",
+		     sizeof("reference___initcall")-1))
+		return;
+
+	/* check whitelist - we may ignore it */
+	if (secref_whitelist(mismatch,
+			     fromsec, fromsym, tosec, tosym)) {
+		report_sec_mismatch(modname, mismatch,
+				    fromsec, r->r_offset, fromsym,
+				    is_function(from), tosec, tosym,
+				    is_function(to));
+	}
+}
+
+static void check_section_mismatch(const char *modname, struct elf_info *elf,
+				   Elf_Rela *r, Elf_Sym *sym, const char *fromsec)
+{
+	const char *tosec = sec_name(elf, get_secindex(elf, sym));;
+	const struct sectioncheck *mismatch = section_mismatch(fromsec, tosec);
+
 	if (mismatch) {
-		Elf_Sym *to;
-		Elf_Sym *from;
-		const char *tosym;
-		const char *fromsym;
-
-		from = find_elf_symbol2(elf, r->r_offset, fromsec);
-		fromsym = sym_name(elf, from);
-		to = find_elf_symbol(elf, r->r_addend, sym);
-		tosym = sym_name(elf, to);
-
-		if (!strncmp(fromsym, "reference___initcall",
-				sizeof("reference___initcall")-1))
-			return;
-
-		/* check whitelist - we may ignore it */
-		if (secref_whitelist(mismatch,
-					fromsec, fromsym, tosec, tosym)) {
-			report_sec_mismatch(modname, mismatch,
-			   fromsec, r->r_offset, fromsym,
-			   is_function(from), tosec, tosym,
-			   is_function(to));
-		}
+		if (mismatch->handler)
+			mismatch->handler(modname, elf,  mismatch,
+					  r, sym, fromsec);
+		else
+			default_mismatch_handler(modname, elf, mismatch,
+						 r, sym, fromsec);
 	}
 }
 
-- 
2.0.5

[PATCH 4/7] modpost: factorize symbol pretty print in get_pretty_name().

[Quentin Casasnovas]

Signed-off-by: Quentin Casasnovas <quentin.casasnovas@oracle.com>
---
 scripts/mod/modpost.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 0f48f8b..c69681e 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1270,6 +1270,15 @@ static void print_section_list(const char * const list[20])
 	fprintf(stderr, "\n");
 }
 
+static inline void get_pretty_name(int is_func, const char** name, const char** name_p)
+{
+	switch (is_func) {
+	case 0:	*name = "variable"; *name_p = ""; break;
+	case 1:	*name = "function"; *name_p = "()"; break;
+	default: *name = "(unknown reference)"; *name_p = ""; break;
+	}
+}
+
 /*
  * Print a warning about a section mismatch.
  * Try to find symbols near it so user can find it.
@@ -1289,21 +1298,13 @@ static void report_sec_mismatch(const char *modname,
 	char *prl_from;
 	char *prl_to;
 
-	switch (from_is_func) {
-	case 0: from = "variable"; from_p = "";   break;
-	case 1: from = "function"; from_p = "()"; break;
-	default: from = "(unknown reference)"; from_p = ""; break;
-	}
-	switch (to_is_func) {
-	case 0: to = "variable"; to_p = "";   break;
-	case 1: to = "function"; to_p = "()"; break;
-	default: to = "(unknown reference)"; to_p = ""; break;
-	}
-
 	sec_mismatch_count++;
 	if (!sec_mismatch_verbose)
 		return;
 
+	get_pretty_name(from_is_func, &from, &from_p);
+	get_pretty_name(to_is_func, &to, &to_p);
+
 	warn("%s(%s+0x%llx): Section mismatch in reference from the %s %s%s "
 	     "to the %s %s:%s%s\n",
 	     modname, fromsec, fromaddr, from, fromsym, from_p, to, tosec,
-- 
2.0.5

[PATCH 5/7] modpost: mismatch_handler: retrieve tosym information only when needed.

[Quentin Casasnovas]

Signed-off-by: Quentin Casasnovas <quentin.casasnovas@oracle.com>
---
 scripts/mod/modpost.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index c69681e..bf0cf81 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1432,16 +1432,17 @@ static void default_mismatch_handler(const char *modname, struct elf_info *elf,
 	const char *tosym;
 	const char *fromsym;
 
-	tosec = sec_name(elf, get_secindex(elf, sym));
 	from = find_elf_symbol2(elf, r->r_offset, fromsec);
 	fromsym = sym_name(elf, from);
-	to = find_elf_symbol(elf, r->r_addend, sym);
-	tosym = sym_name(elf, to);
 
 	if (!strncmp(fromsym, "reference___initcall",
 		     sizeof("reference___initcall")-1))
 		return;
 
+	tosec = sec_name(elf, get_secindex(elf, sym));
+	to = find_elf_symbol(elf, r->r_addend, sym);
+	tosym = sym_name(elf, to);
+
 	/* check whitelist - we may ignore it */
 	if (secref_whitelist(mismatch,
 			     fromsec, fromsym, tosec, tosym)) {
-- 
2.0.5

[PATCH 6/7] scripts: add check_extable.sh script.

[Quentin Casasnovas]

This shell script can be used to sanity check the __ex_table section on an
object file, making sure the relocations in there are pointing to valid
executable sections.  If it finds some suspicious relocations, it'll use
addr2line to try and dump where this is coming from.

This works best with CONFIG_DEBUG_INFO.

Signed-off-by: Quentin Casasnovas <quentin.casasnovas@oracle.com>
---
 scripts/check_extable.sh | 146 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 146 insertions(+)
 create mode 100755 scripts/check_extable.sh

diff --git a/scripts/check_extable.sh b/scripts/check_extable.sh
new file mode 100755
index 0000000..0fb6b1c
--- /dev/null
+++ b/scripts/check_extable.sh
@@ -0,0 +1,146 @@
+#! /bin/bash
+# (c) 2015, Quentin Casasnovas <quentin.casasnovas@oracle.com>
+
+obj=$1
+
+file ${obj} | grep -q ELF || (echo "${obj} is not and ELF file." 1>&2 ; exit 0)
+
+# Bail out early if there isn't an __ex_table section in this object file.
+objdump -hj __ex_table ${obj} 2> /dev/null > /dev/null
+[ $? -ne 0 ] && exit 0
+
+white_list=.text,.fixup
+
+suspicious_relocs=$(objdump -rj __ex_table ${obj}  | tail -n +6 |
+			grep -v $(eval echo -e{${white_list}}) | awk '{print $3}')
+
+# No suspicious relocs in __ex_table, jobs a good'un
+[ -z "${suspicious_relocs}" ] && exit 0
+
+
+# After this point, something is seriously wrong since we just found out we
+# have some relocations in __ex_table which point to sections which aren't
+# white listed.  If you're adding a new section in the Linux kernel, and
+# you're expecting this section to contain code which can fault (i.e. the
+# __ex_table relocation to your new section is expected), simply add your
+# new section to the white_list variable above.  If not, you're probably
+# doing something wrong and the rest of this code is just trying to print
+# you more information about it.
+
+function find_section_offset_from_symbol()
+{
+    eval $(objdump -t ${obj} | grep ${1} | sed 's/\([0-9a-f]\+\) .\{7\} \([^ \t]\+\).*/section="\2"; section_offset="0x\1" /')
+
+    # addr2line takes addresses in hexadecimal...
+    section_offset=$(printf "0x%016x" $(( ${section_offset} + $2 )) )
+}
+
+function find_symbol_and_offset_from_reloc()
+{
+    # Extract symbol and offset from the objdump output
+    eval $(echo $reloc | sed 's/\([^+]\+\)+\?\(0x[0-9a-f]\+\)\?/symbol="\1"; symbol_offset="\2"/')
+
+    # When the relocation points to the begining of a symbol or section, it
+    # won't print the offset since it is zero.
+    if [ -z "${symbol_offset}" ]; then
+	symbol_offset=0x0
+    fi
+}
+
+function find_alt_replacement_target()
+{
+    # The target of the .altinstr_replacement is the relocation just before
+    # the .altinstr_replacement one.
+    eval $(objdump -rj .altinstructions ${obj} | grep -B1 "${section}+${section_offset}" | head -n1 | awk '{print $3}' |
+	   sed 's/\([^+]\+\)+\(0x[0-9a-f]\+\)/alt_target_section="\1"; alt_target_offset="\2"/')
+}
+
+function handle_alt_replacement_reloc()
+{
+    # This will define alt_target_section and alt_target_section_offset
+    find_alt_replacement_target ${section} ${section_offset}
+
+    echo "Error: found a reference to .altinstr_replacement in __ex_table:"
+    addr2line -fip -j ${alt_target_section} -e ${obj} ${alt_target_offset} | awk '{print "\t" $0}'
+
+    error=true
+}
+
+function is_executable_section()
+{
+    objdump -hwj ${section} ${obj} | grep -q CODE
+    return $?
+}
+
+function handle_suspicious_generic_reloc()
+{
+    if is_executable_section ${section}; then
+	# We've got a relocation to a non white listed _executable_
+	# section, print a warning so the developper adds the section to
+	# the white list or fix his code.  We try to pretty-print the file
+	# and line number where that relocation was added.
+	echo "Warning: found a reference to section \"${section}\" in __ex_table:"
+	addr2line -fip -j ${section} -e ${obj} ${section_offset} | awk '{print "\t" $0}'
+    else
+	# Something is definitively wrong here since we've got a relocation
+	# to a non-executable section, there's no way this would ever be
+	# running in the kernel.
+	echo "Error: found a reference to non-executable section \"${section}\" in __ex_table at offset ${section_offset}"
+	error=true
+    fi
+}
+
+function handle_suspicious_reloc()
+{
+    case "${section}" in
+	".altinstr_replacement")
+	    handle_alt_replacement_reloc ${section} ${section_offset}
+	    ;;
+	*)
+	    handle_suspicious_generic_reloc ${section} ${section_offset}
+	    ;;
+    esac
+}
+
+function diagnose()
+{
+
+    for reloc in ${suspicious_relocs}; do
+	# Let's find out where the target of the relocation in __ex_table
+	# is, this will define ${symbol} and ${symbol_offset}
+	find_symbol_and_offset_from_reloc ${reloc}
+
+	# When there's a global symbol at the place of the relocation,
+	# objdump will use it instead of giving us a section+offset, so
+	# let's find out which section is this symbol in and the total
+	# offset withing that section.
+	find_section_offset_from_symbol ${symbol} ${symbol_offset}
+
+	# In this case objdump was presenting us with a reloc to a symbol
+	# rather than a section. Now that we've got the actual section,
+	# we can skip it if it's in the white_list.
+	if [ -z "$( echo $section | grep -v $(eval echo -e{${white_list}}))" ]; then
+	    continue;
+	fi
+
+	# Will either print a warning if the relocation happens to be in a
+	# section we do not know but has executable bit set, or error out.
+	handle_suspicious_reloc
+    done
+}
+
+function check_debug_info() {
+    objdump -hj .debug_info ${obj} 2> /dev/null > /dev/null ||
+	echo -e "${obj} does not contain debug information, the addr2line output will be limited.\n" \
+	     "Recompile ${obj} with CONFIG_DEBUG_INFO to get a more useful output."
+}
+
+check_debug_info
+
+diagnose
+
+if [ "${error}" ]; then
+    exit 1
+fi
+
+exit 0
-- 
2.0.5

[PATCH 7/7] modpost: handle relocations mismatch in __ex_table.

[Quentin Casasnovas]

__ex_table is a simple table section where each entry is a pair of
addresses - the first address is an address which can fault in kernel
space, and the second address points to where the kernel should jump to
when handling that fault.  This is how copy_from_user() does not crash the
kernel if userspace gives a borked pointer for example.

If one of these addresses point to a non-executable section, something is
seriously wrong since it either means the kernel will never fault from
there or it will not be able to jump to there.  As both cases are serious
enough, we simply error out in these cases so the build fails and the
developper has to fix the issue.

In case the section is executable, but it isn't referenced in our list of
authorized sections to point to from __ex_table, we just dump a warning
giving more information about it.  We do this in case the new section is
executable but isn't supposed to be executed by the kernel.  This happened
with .altinstr_replacement, which is executable but is only used to copy
instructions from - we should never have our instruction pointer pointing
in .altinstr_replacement.  Admitedly, a proper fix in that case would be to
just set .altinstr_replacement NX, but we need to warn about future cases
like this.

Signed-off-by: Quentin Casasnovas <quentin.casasnovas@oracle.com>
---
 scripts/mod/modpost.c | 141 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 141 insertions(+)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index bf0cf81..dfe9c3c 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -875,6 +875,8 @@ static void check_section(const char *modname, struct elf_info *elf,
 #define DATA_SECTIONS ".data", ".data.rel"
 #define TEXT_SECTIONS ".text", ".text.unlikely", ".sched.text", \
 		".kprobes.text"
+#define OTHER_TEXT_SECTIONS ".ref.text", ".head.text", ".spinlock.text", \
+		".fixup", ".entry.text"
 
 #define INIT_SECTIONS      ".init.*"
 #define MEM_INIT_SECTIONS  ".meminit.*"
@@ -882,6 +884,9 @@ static void check_section(const char *modname, struct elf_info *elf,
 #define EXIT_SECTIONS      ".exit.*"
 #define MEM_EXIT_SECTIONS  ".memexit.*"
 
+#define ALL_TEXT_SECTIONS  ALL_INIT_TEXT_SECTIONS, ALL_EXIT_TEXT_SECTIONS, \
+		TEXT_SECTIONS, OTHER_TEXT_SECTIONS
+
 /* init data sections */
 static const char *const init_data_sections[] =
 	{ ALL_INIT_DATA_SECTIONS, NULL };
@@ -922,6 +927,7 @@ enum mismatch {
 	ANY_INIT_TO_ANY_EXIT,
 	ANY_EXIT_TO_ANY_INIT,
 	EXPORT_TO_INIT_EXIT,
+	EXTABLE_TO_NON_TEXT,
 };
 
 struct sectioncheck {
@@ -936,6 +942,11 @@ struct sectioncheck {
 
 };
 
+static void extable_mismatch_handler(const char *modname, struct elf_info *elf,
+				     const struct sectioncheck* const mismatch,
+				     Elf_Rela *r, Elf_Sym *sym,
+				     const char *fromsec);
+
 static const struct sectioncheck sectioncheck[] = {
 /* Do not reference init/exit code/data from
  * normal code and data
@@ -1013,6 +1024,16 @@ static const struct sectioncheck sectioncheck[] = {
 	.bad_tosec = { INIT_SECTIONS, EXIT_SECTIONS, NULL },
 	.mismatch = EXPORT_TO_INIT_EXIT,
 	.symbol_white_list = { DEFAULT_SYMBOL_WHITE_LIST, NULL },
+},
+{
+	.fromsec = { "__ex_table", NULL },
+	/* If you're adding any new black-listed sections in here, consider
+	 * adding a special 'printer' for them in scripts/check_extable.
+	 */
+	.bad_tosec = { ".altinstr_replacement", NULL },
+	.good_tosec = {ALL_TEXT_SECTIONS , NULL},
+	.mismatch = EXTABLE_TO_NON_TEXT,
+	.handler = extable_mismatch_handler,
 }
 };
 
@@ -1418,6 +1439,10 @@ static void report_sec_mismatch(const char *modname,
 		tosym, prl_to, prl_to, tosym);
 		free(prl_to);
 		break;
+	case EXTABLE_TO_NON_TEXT:
+		fatal("There's a special handler for this mismatch type, "
+		      "we should never get here.");
+		break;
 	}
 	fprintf(stderr, "\n");
 }
@@ -1453,6 +1478,120 @@ static void default_mismatch_handler(const char *modname, struct elf_info *elf,
 	}
 }
 
+static int is_executable_section(struct elf_info* elf, unsigned int section_index)
+{
+	if (section_index > elf->num_sections)
+		fatal("section_index is outside elf->num_sections!\n");
+
+	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,
+				    const void* start, const void* cur)
+{
+	/*
+	 * 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 && cur == start + 1 &&
+	    strcmp("__ex_table", sec) == 0)
+		extable_entry_size = r->r_offset * 2;
+}
+static inline bool is_extable_fault_address(Elf_Rela *r)
+{
+	if (!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));
+}
+
+static void report_extable_warnings(const char* modname, struct elf_info* elf,
+				    const struct sectioncheck* const mismatch,
+				    Elf_Rela* r, Elf_Sym* sym,
+				    const char* fromsec, const char* tosec)
+{
+	Elf_Sym* fromsym = find_elf_symbol2(elf, r->r_offset, fromsec);
+	const char* fromsym_name = sym_name(elf, fromsym);
+	Elf_Sym* tosym = find_elf_symbol(elf, r->r_addend, sym);
+	const char* tosym_name = sym_name(elf, tosym);
+	const char* from_pretty_name;
+	const char* from_pretty_name_p;
+	const char* to_pretty_name;
+	const char* to_pretty_name_p;
+
+	get_pretty_name(is_function(fromsym),
+			&from_pretty_name, &from_pretty_name_p);
+	get_pretty_name(is_function(tosym),
+			&to_pretty_name, &to_pretty_name_p);
+
+	warn("%s(%s+0x%lx): Section mismatch in reference"
+	     " from the %s %s%s to the %s %s:%s%s\n",
+	     modname, fromsec, r->r_offset, from_pretty_name,
+	     fromsym_name, from_pretty_name_p,
+	     to_pretty_name, tosec, tosym_name, to_pretty_name_p);
+
+	if (!match(tosec, mismatch->bad_tosec) &&
+	    is_executable_section(elf, get_secindex(elf, sym)))
+		fprintf(stderr,
+			"The relocation at %s+0x%lx references\n"
+			"section \"%s\" which is not in the list of\n"
+			"authorized sections.  If you're adding a new section\n"
+			"and/or if this reference is valid, add \"%s\" to the\n"
+			"list of authorized sections to jump to on fault.\n"
+			"This can be achieved by adding \"%s\" to \n"
+			"OTHER_TEXT_SECTIONS in scripts/mod/modpost.c.\n",
+			fromsec, r->r_offset, tosec, tosec, tosec);
+}
+
+static void extable_mismatch_handler(const char* modname, struct elf_info *elf,
+				     const struct sectioncheck* const mismatch,
+				     Elf_Rela* r, Elf_Sym* sym,
+				     const char *fromsec)
+{
+	const char* tosec = sec_name(elf, get_secindex(elf, sym));
+
+	sec_mismatch_count++;
+
+	if (sec_mismatch_verbose)
+		report_extable_warnings(modname, elf, mismatch, r, sym,
+					fromsec, tosec);
+
+	if (match(tosec, mismatch->bad_tosec))
+		fatal("The relocation at %s+0x%lx references\n"
+		      "section \"%s\" which is black-listed.\n"
+		      "Something is seriously wrong and should be fixed.\n"
+		      "You might get more information about where this is\n"
+		      "coming from by using scripts/check_extable.sh %s\n",
+		      fromsec, 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, 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, r->r_offset, tosec);
+	}
+}
+
 static void check_section_mismatch(const char *modname, struct elf_info *elf,
 				   Elf_Rela *r, Elf_Sym *sym, const char *fromsec)
 {
@@ -1605,6 +1744,7 @@ static void section_rela(const char *modname, struct elf_info *elf,
 		/* Skip special sections */
 		if (is_shndx_special(sym->st_shndx))
 			continue;
+		find_extable_entry_size(fromsec, &r, start, rela);
 		check_section_mismatch(modname, elf, &r, sym, fromsec);
 	}
 }
@@ -1663,6 +1803,7 @@ static void section_rel(const char *modname, struct elf_info *elf,
 		/* Skip special sections */
 		if (is_shndx_special(sym->st_shndx))
 			continue;
+		find_extable_entry_size(fromsec, &r, start, rel);
 		check_section_mismatch(modname, elf, &r, sym, fromsec);
 	}
 }
-- 
2.0.5

Re: [PATCH 1/7] modpost: add strict white-listing when referencing sections.

[Linus Torvalds]

On Tue, Mar 17, 2015 at 5:39 AM, Quentin Casasnovas
<quentin.casasnovas@oracle.com> wrote:
> Prints a warning when a section references a section outside a strict
> white-list.  This will be useful to print a warning if __ex_table
> references a non-executable section.

So I think all of these patches are ok, but you should probably add a
few more people explicitly to the cc.

Probably Rusty (because modpost) and MIchal (because kbuild). The
kernel mailing list is so high-volume and untargeted that it's usually
better as a "archival cc" than as a way to find people to comment and
merge the patches.

                        Linus

Re: [PATCH 2/7] modpost: add .sched.text and .kprobes.text to the TEXT_SECTIONS list.

[Quentin Casasnovas]

Adding Rusty and Michal to CC.

On Tue, Mar 17, 2015 at 01:39:57PM +0100, Quentin Casasnovas wrote:
> .sched.text and .kprobes.text should behave exactly like .text with regards
> to how we should warn about referencing sections which might get discarded
> at runtime.
> 
> Signed-off-by: Quentin Casasnovas <quentin.casasnovas@oracle.com>
> ---
>  scripts/mod/modpost.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 7094a57..8cef46b 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -873,7 +873,8 @@ static void check_section(const char *modname, struct elf_info *elf,
>  #define ALL_EXIT_SECTIONS EXIT_SECTIONS, ALL_XXXEXIT_SECTIONS
>  
>  #define DATA_SECTIONS ".data", ".data.rel"
> -#define TEXT_SECTIONS ".text", ".text.unlikely"
> +#define TEXT_SECTIONS ".text", ".text.unlikely", ".sched.text", \
> +		".kprobes.text"
>  
>  #define INIT_SECTIONS      ".init.*"
>  #define MEM_INIT_SECTIONS  ".meminit.*"
> -- 
> 2.0.5
> 

Re: [PATCH 3/7] modpost: add handler function pointer to sectioncheck.

[Quentin Casasnovas]

Adding Rusty and Michal to CC.

On Tue, Mar 17, 2015 at 01:39:58PM +0100, Quentin Casasnovas wrote:
> This will be useful when we want to have special handlers which need to go
> through more hops to print useful information to the user.
> 
> Signed-off-by: Quentin Casasnovas <quentin.casasnovas@oracle.com>
> ---
>  scripts/mod/modpost.c | 68 +++++++++++++++++++++++++++++++--------------------
>  1 file changed, 42 insertions(+), 26 deletions(-)
> 
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 8cef46b..0f48f8b 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -930,6 +930,10 @@ struct sectioncheck {
>  	const char *good_tosec[20];
>  	enum mismatch mismatch;
>  	const char *symbol_white_list[20];
> +	void (*handler)(const char *modname, struct elf_info *elf,
> +			const struct sectioncheck* const mismatch,
> +			Elf_Rela *r, Elf_Sym *sym, const char *fromsec);
> +
>  };
>  
>  static const struct sectioncheck sectioncheck[] = {
> @@ -1417,37 +1421,49 @@ static void report_sec_mismatch(const char *modname,
>  	fprintf(stderr, "\n");
>  }
>  
> -static void check_section_mismatch(const char *modname, struct elf_info *elf,
> -				   Elf_Rela *r, Elf_Sym *sym, const char *fromsec)
> +static void default_mismatch_handler(const char *modname, struct elf_info *elf,
> +				     const struct sectioncheck* const mismatch,
> +				     Elf_Rela *r, Elf_Sym *sym, const char *fromsec)
>  {
>  	const char *tosec;
> -	const struct sectioncheck *mismatch;
> +	Elf_Sym *to;
> +	Elf_Sym *from;
> +	const char *tosym;
> +	const char *fromsym;
>  
>  	tosec = sec_name(elf, get_secindex(elf, sym));
> -	mismatch = section_mismatch(fromsec, tosec);
> +	from = find_elf_symbol2(elf, r->r_offset, fromsec);
> +	fromsym = sym_name(elf, from);
> +	to = find_elf_symbol(elf, r->r_addend, sym);
> +	tosym = sym_name(elf, to);
> +
> +	if (!strncmp(fromsym, "reference___initcall",
> +		     sizeof("reference___initcall")-1))
> +		return;
> +
> +	/* check whitelist - we may ignore it */
> +	if (secref_whitelist(mismatch,
> +			     fromsec, fromsym, tosec, tosym)) {
> +		report_sec_mismatch(modname, mismatch,
> +				    fromsec, r->r_offset, fromsym,
> +				    is_function(from), tosec, tosym,
> +				    is_function(to));
> +	}
> +}
> +
> +static void check_section_mismatch(const char *modname, struct elf_info *elf,
> +				   Elf_Rela *r, Elf_Sym *sym, const char *fromsec)
> +{
> +	const char *tosec = sec_name(elf, get_secindex(elf, sym));;
> +	const struct sectioncheck *mismatch = section_mismatch(fromsec, tosec);
> +
>  	if (mismatch) {
> -		Elf_Sym *to;
> -		Elf_Sym *from;
> -		const char *tosym;
> -		const char *fromsym;
> -
> -		from = find_elf_symbol2(elf, r->r_offset, fromsec);
> -		fromsym = sym_name(elf, from);
> -		to = find_elf_symbol(elf, r->r_addend, sym);
> -		tosym = sym_name(elf, to);
> -
> -		if (!strncmp(fromsym, "reference___initcall",
> -				sizeof("reference___initcall")-1))
> -			return;
> -
> -		/* check whitelist - we may ignore it */
> -		if (secref_whitelist(mismatch,
> -					fromsec, fromsym, tosec, tosym)) {
> -			report_sec_mismatch(modname, mismatch,
> -			   fromsec, r->r_offset, fromsym,
> -			   is_function(from), tosec, tosym,
> -			   is_function(to));
> -		}
> +		if (mismatch->handler)
> +			mismatch->handler(modname, elf,  mismatch,
> +					  r, sym, fromsec);
> +		else
> +			default_mismatch_handler(modname, elf, mismatch,
> +						 r, sym, fromsec);
>  	}
>  }
>  
> -- 
> 2.0.5
> 

Re: [PATCH 4/7] modpost: factorize symbol pretty print in get_pretty_name().

[Quentin Casasnovas]

Adding Rusty and Michal to CC.

On Tue, Mar 17, 2015 at 01:39:59PM +0100, Quentin Casasnovas wrote:
> Signed-off-by: Quentin Casasnovas <quentin.casasnovas@oracle.com>
> ---
>  scripts/mod/modpost.c | 23 ++++++++++++-----------
>  1 file changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 0f48f8b..c69681e 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -1270,6 +1270,15 @@ static void print_section_list(const char * const list[20])
>  	fprintf(stderr, "\n");
>  }
>  
> +static inline void get_pretty_name(int is_func, const char** name, const char** name_p)
> +{
> +	switch (is_func) {
> +	case 0:	*name = "variable"; *name_p = ""; break;
> +	case 1:	*name = "function"; *name_p = "()"; break;
> +	default: *name = "(unknown reference)"; *name_p = ""; break;
> +	}
> +}
> +
>  /*
>   * Print a warning about a section mismatch.
>   * Try to find symbols near it so user can find it.
> @@ -1289,21 +1298,13 @@ static void report_sec_mismatch(const char *modname,
>  	char *prl_from;
>  	char *prl_to;
>  
> -	switch (from_is_func) {
> -	case 0: from = "variable"; from_p = "";   break;
> -	case 1: from = "function"; from_p = "()"; break;
> -	default: from = "(unknown reference)"; from_p = ""; break;
> -	}
> -	switch (to_is_func) {
> -	case 0: to = "variable"; to_p = "";   break;
> -	case 1: to = "function"; to_p = "()"; break;
> -	default: to = "(unknown reference)"; to_p = ""; break;
> -	}
> -
>  	sec_mismatch_count++;
>  	if (!sec_mismatch_verbose)
>  		return;
>  
> +	get_pretty_name(from_is_func, &from, &from_p);
> +	get_pretty_name(to_is_func, &to, &to_p);
> +
>  	warn("%s(%s+0x%llx): Section mismatch in reference from the %s %s%s "
>  	     "to the %s %s:%s%s\n",
>  	     modname, fromsec, fromaddr, from, fromsym, from_p, to, tosec,
> -- 
> 2.0.5
> 

Re: [PATCH 5/7] modpost: mismatch_handler: retrieve tosym information only when needed.

[Quentin Casasnovas]

Adding Rusty and Michal to CC.

On Tue, Mar 17, 2015 at 01:40:00PM +0100, Quentin Casasnovas wrote:
> Signed-off-by: Quentin Casasnovas <quentin.casasnovas@oracle.com>
> ---
>  scripts/mod/modpost.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index c69681e..bf0cf81 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -1432,16 +1432,17 @@ static void default_mismatch_handler(const char *modname, struct elf_info *elf,
>  	const char *tosym;
>  	const char *fromsym;
>  
> -	tosec = sec_name(elf, get_secindex(elf, sym));
>  	from = find_elf_symbol2(elf, r->r_offset, fromsec);
>  	fromsym = sym_name(elf, from);
> -	to = find_elf_symbol(elf, r->r_addend, sym);
> -	tosym = sym_name(elf, to);
>  
>  	if (!strncmp(fromsym, "reference___initcall",
>  		     sizeof("reference___initcall")-1))
>  		return;
>  
> +	tosec = sec_name(elf, get_secindex(elf, sym));
> +	to = find_elf_symbol(elf, r->r_addend, sym);
> +	tosym = sym_name(elf, to);
> +
>  	/* check whitelist - we may ignore it */
>  	if (secref_whitelist(mismatch,
>  			     fromsec, fromsym, tosec, tosym)) {
> -- 
> 2.0.5
> 

Re: [PATCH 6/7] scripts: add check_extable.sh script.

[Quentin Casasnovas]

Adding Rusty and Michal to CC.

On Tue, Mar 17, 2015 at 01:40:01PM +0100, Quentin Casasnovas wrote:
> This shell script can be used to sanity check the __ex_table section on an
> object file, making sure the relocations in there are pointing to valid
> executable sections.  If it finds some suspicious relocations, it'll use
> addr2line to try and dump where this is coming from.
> 
> This works best with CONFIG_DEBUG_INFO.
> 
> Signed-off-by: Quentin Casasnovas <quentin.casasnovas@oracle.com>
> ---
>  scripts/check_extable.sh | 146 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 146 insertions(+)
>  create mode 100755 scripts/check_extable.sh
> 
> diff --git a/scripts/check_extable.sh b/scripts/check_extable.sh
> new file mode 100755
> index 0000000..0fb6b1c
> --- /dev/null
> +++ b/scripts/check_extable.sh
> @@ -0,0 +1,146 @@
> +#! /bin/bash
> +# (c) 2015, Quentin Casasnovas <quentin.casasnovas@oracle.com>
> +
> +obj=$1
> +
> +file ${obj} | grep -q ELF || (echo "${obj} is not and ELF file." 1>&2 ; exit 0)
> +
> +# Bail out early if there isn't an __ex_table section in this object file.
> +objdump -hj __ex_table ${obj} 2> /dev/null > /dev/null
> +[ $? -ne 0 ] && exit 0
> +
> +white_list=.text,.fixup
> +
> +suspicious_relocs=$(objdump -rj __ex_table ${obj}  | tail -n +6 |
> +			grep -v $(eval echo -e{${white_list}}) | awk '{print $3}')
> +
> +# No suspicious relocs in __ex_table, jobs a good'un
> +[ -z "${suspicious_relocs}" ] && exit 0
> +
> +
> +# After this point, something is seriously wrong since we just found out we
> +# have some relocations in __ex_table which point to sections which aren't
> +# white listed.  If you're adding a new section in the Linux kernel, and
> +# you're expecting this section to contain code which can fault (i.e. the
> +# __ex_table relocation to your new section is expected), simply add your
> +# new section to the white_list variable above.  If not, you're probably
> +# doing something wrong and the rest of this code is just trying to print
> +# you more information about it.
> +
> +function find_section_offset_from_symbol()
> +{
> +    eval $(objdump -t ${obj} | grep ${1} | sed 's/\([0-9a-f]\+\) .\{7\} \([^ \t]\+\).*/section="\2"; section_offset="0x\1" /')
> +
> +    # addr2line takes addresses in hexadecimal...
> +    section_offset=$(printf "0x%016x" $(( ${section_offset} + $2 )) )
> +}
> +
> +function find_symbol_and_offset_from_reloc()
> +{
> +    # Extract symbol and offset from the objdump output
> +    eval $(echo $reloc | sed 's/\([^+]\+\)+\?\(0x[0-9a-f]\+\)\?/symbol="\1"; symbol_offset="\2"/')
> +
> +    # When the relocation points to the begining of a symbol or section, it
> +    # won't print the offset since it is zero.
> +    if [ -z "${symbol_offset}" ]; then
> +	symbol_offset=0x0
> +    fi
> +}
> +
> +function find_alt_replacement_target()
> +{
> +    # The target of the .altinstr_replacement is the relocation just before
> +    # the .altinstr_replacement one.
> +    eval $(objdump -rj .altinstructions ${obj} | grep -B1 "${section}+${section_offset}" | head -n1 | awk '{print $3}' |
> +	   sed 's/\([^+]\+\)+\(0x[0-9a-f]\+\)/alt_target_section="\1"; alt_target_offset="\2"/')
> +}
> +
> +function handle_alt_replacement_reloc()
> +{
> +    # This will define alt_target_section and alt_target_section_offset
> +    find_alt_replacement_target ${section} ${section_offset}
> +
> +    echo "Error: found a reference to .altinstr_replacement in __ex_table:"
> +    addr2line -fip -j ${alt_target_section} -e ${obj} ${alt_target_offset} | awk '{print "\t" $0}'
> +
> +    error=true
> +}
> +
> +function is_executable_section()
> +{
> +    objdump -hwj ${section} ${obj} | grep -q CODE
> +    return $?
> +}
> +
> +function handle_suspicious_generic_reloc()
> +{
> +    if is_executable_section ${section}; then
> +	# We've got a relocation to a non white listed _executable_
> +	# section, print a warning so the developper adds the section to
> +	# the white list or fix his code.  We try to pretty-print the file
> +	# and line number where that relocation was added.
> +	echo "Warning: found a reference to section \"${section}\" in __ex_table:"
> +	addr2line -fip -j ${section} -e ${obj} ${section_offset} | awk '{print "\t" $0}'
> +    else
> +	# Something is definitively wrong here since we've got a relocation
> +	# to a non-executable section, there's no way this would ever be
> +	# running in the kernel.
> +	echo "Error: found a reference to non-executable section \"${section}\" in __ex_table at offset ${section_offset}"
> +	error=true
> +    fi
> +}
> +
> +function handle_suspicious_reloc()
> +{
> +    case "${section}" in
> +	".altinstr_replacement")
> +	    handle_alt_replacement_reloc ${section} ${section_offset}
> +	    ;;
> +	*)
> +	    handle_suspicious_generic_reloc ${section} ${section_offset}
> +	    ;;
> +    esac
> +}
> +
> +function diagnose()
> +{
> +
> +    for reloc in ${suspicious_relocs}; do
> +	# Let's find out where the target of the relocation in __ex_table
> +	# is, this will define ${symbol} and ${symbol_offset}
> +	find_symbol_and_offset_from_reloc ${reloc}
> +
> +	# When there's a global symbol at the place of the relocation,
> +	# objdump will use it instead of giving us a section+offset, so
> +	# let's find out which section is this symbol in and the total
> +	# offset withing that section.
> +	find_section_offset_from_symbol ${symbol} ${symbol_offset}
> +
> +	# In this case objdump was presenting us with a reloc to a symbol
> +	# rather than a section. Now that we've got the actual section,
> +	# we can skip it if it's in the white_list.
> +	if [ -z "$( echo $section | grep -v $(eval echo -e{${white_list}}))" ]; then
> +	    continue;
> +	fi
> +
> +	# Will either print a warning if the relocation happens to be in a
> +	# section we do not know but has executable bit set, or error out.
> +	handle_suspicious_reloc
> +    done
> +}
> +
> +function check_debug_info() {
> +    objdump -hj .debug_info ${obj} 2> /dev/null > /dev/null ||
> +	echo -e "${obj} does not contain debug information, the addr2line output will be limited.\n" \
> +	     "Recompile ${obj} with CONFIG_DEBUG_INFO to get a more useful output."
> +}
> +
> +check_debug_info
> +
> +diagnose
> +
> +if [ "${error}" ]; then
> +    exit 1
> +fi
> +
> +exit 0
> -- 
> 2.0.5
> 

Re: [PATCH 7/7] modpost: handle relocations mismatch in __ex_table.

[Quentin Casasnovas]

Adding Rusty and Michal to CC.

On Tue, Mar 17, 2015 at 01:40:02PM +0100, Quentin Casasnovas wrote:
> __ex_table is a simple table section where each entry is a pair of
> addresses - the first address is an address which can fault in kernel
> space, and the second address points to where the kernel should jump to
> when handling that fault.  This is how copy_from_user() does not crash the
> kernel if userspace gives a borked pointer for example.
> 
> If one of these addresses point to a non-executable section, something is
> seriously wrong since it either means the kernel will never fault from
> there or it will not be able to jump to there.  As both cases are serious
> enough, we simply error out in these cases so the build fails and the
> developper has to fix the issue.
> 
> In case the section is executable, but it isn't referenced in our list of
> authorized sections to point to from __ex_table, we just dump a warning
> giving more information about it.  We do this in case the new section is
> executable but isn't supposed to be executed by the kernel.  This happened
> with .altinstr_replacement, which is executable but is only used to copy
> instructions from - we should never have our instruction pointer pointing
> in .altinstr_replacement.  Admitedly, a proper fix in that case would be to
> just set .altinstr_replacement NX, but we need to warn about future cases
> like this.
> 
> Signed-off-by: Quentin Casasnovas <quentin.casasnovas@oracle.com>
> ---
>  scripts/mod/modpost.c | 141 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 141 insertions(+)
> 
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index bf0cf81..dfe9c3c 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -875,6 +875,8 @@ static void check_section(const char *modname, struct elf_info *elf,
>  #define DATA_SECTIONS ".data", ".data.rel"
>  #define TEXT_SECTIONS ".text", ".text.unlikely", ".sched.text", \
>  		".kprobes.text"
> +#define OTHER_TEXT_SECTIONS ".ref.text", ".head.text", ".spinlock.text", \
> +		".fixup", ".entry.text"
>  
>  #define INIT_SECTIONS      ".init.*"
>  #define MEM_INIT_SECTIONS  ".meminit.*"
> @@ -882,6 +884,9 @@ static void check_section(const char *modname, struct elf_info *elf,
>  #define EXIT_SECTIONS      ".exit.*"
>  #define MEM_EXIT_SECTIONS  ".memexit.*"
>  
> +#define ALL_TEXT_SECTIONS  ALL_INIT_TEXT_SECTIONS, ALL_EXIT_TEXT_SECTIONS, \
> +		TEXT_SECTIONS, OTHER_TEXT_SECTIONS
> +
>  /* init data sections */
>  static const char *const init_data_sections[] =
>  	{ ALL_INIT_DATA_SECTIONS, NULL };
> @@ -922,6 +927,7 @@ enum mismatch {
>  	ANY_INIT_TO_ANY_EXIT,
>  	ANY_EXIT_TO_ANY_INIT,
>  	EXPORT_TO_INIT_EXIT,
> +	EXTABLE_TO_NON_TEXT,
>  };
>  
>  struct sectioncheck {
> @@ -936,6 +942,11 @@ struct sectioncheck {
>  
>  };
>  
> +static void extable_mismatch_handler(const char *modname, struct elf_info *elf,
> +				     const struct sectioncheck* const mismatch,
> +				     Elf_Rela *r, Elf_Sym *sym,
> +				     const char *fromsec);
> +
>  static const struct sectioncheck sectioncheck[] = {
>  /* Do not reference init/exit code/data from
>   * normal code and data
> @@ -1013,6 +1024,16 @@ static const struct sectioncheck sectioncheck[] = {
>  	.bad_tosec = { INIT_SECTIONS, EXIT_SECTIONS, NULL },
>  	.mismatch = EXPORT_TO_INIT_EXIT,
>  	.symbol_white_list = { DEFAULT_SYMBOL_WHITE_LIST, NULL },
> +},
> +{
> +	.fromsec = { "__ex_table", NULL },
> +	/* If you're adding any new black-listed sections in here, consider
> +	 * adding a special 'printer' for them in scripts/check_extable.
> +	 */
> +	.bad_tosec = { ".altinstr_replacement", NULL },
> +	.good_tosec = {ALL_TEXT_SECTIONS , NULL},
> +	.mismatch = EXTABLE_TO_NON_TEXT,
> +	.handler = extable_mismatch_handler,
>  }
>  };
>  
> @@ -1418,6 +1439,10 @@ static void report_sec_mismatch(const char *modname,
>  		tosym, prl_to, prl_to, tosym);
>  		free(prl_to);
>  		break;
> +	case EXTABLE_TO_NON_TEXT:
> +		fatal("There's a special handler for this mismatch type, "
> +		      "we should never get here.");
> +		break;
>  	}
>  	fprintf(stderr, "\n");
>  }
> @@ -1453,6 +1478,120 @@ static void default_mismatch_handler(const char *modname, struct elf_info *elf,
>  	}
>  }
>  
> +static int is_executable_section(struct elf_info* elf, unsigned int section_index)
> +{
> +	if (section_index > elf->num_sections)
> +		fatal("section_index is outside elf->num_sections!\n");
> +
> +	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,
> +				    const void* start, const void* cur)
> +{
> +	/*
> +	 * 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 && cur == start + 1 &&
> +	    strcmp("__ex_table", sec) == 0)
> +		extable_entry_size = r->r_offset * 2;
> +}
> +static inline bool is_extable_fault_address(Elf_Rela *r)
> +{
> +	if (!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));
> +}
> +
> +static void report_extable_warnings(const char* modname, struct elf_info* elf,
> +				    const struct sectioncheck* const mismatch,
> +				    Elf_Rela* r, Elf_Sym* sym,
> +				    const char* fromsec, const char* tosec)
> +{
> +	Elf_Sym* fromsym = find_elf_symbol2(elf, r->r_offset, fromsec);
> +	const char* fromsym_name = sym_name(elf, fromsym);
> +	Elf_Sym* tosym = find_elf_symbol(elf, r->r_addend, sym);
> +	const char* tosym_name = sym_name(elf, tosym);
> +	const char* from_pretty_name;
> +	const char* from_pretty_name_p;
> +	const char* to_pretty_name;
> +	const char* to_pretty_name_p;
> +
> +	get_pretty_name(is_function(fromsym),
> +			&from_pretty_name, &from_pretty_name_p);
> +	get_pretty_name(is_function(tosym),
> +			&to_pretty_name, &to_pretty_name_p);
> +
> +	warn("%s(%s+0x%lx): Section mismatch in reference"
> +	     " from the %s %s%s to the %s %s:%s%s\n",
> +	     modname, fromsec, r->r_offset, from_pretty_name,
> +	     fromsym_name, from_pretty_name_p,
> +	     to_pretty_name, tosec, tosym_name, to_pretty_name_p);
> +
> +	if (!match(tosec, mismatch->bad_tosec) &&
> +	    is_executable_section(elf, get_secindex(elf, sym)))
> +		fprintf(stderr,
> +			"The relocation at %s+0x%lx references\n"
> +			"section \"%s\" which is not in the list of\n"
> +			"authorized sections.  If you're adding a new section\n"
> +			"and/or if this reference is valid, add \"%s\" to the\n"
> +			"list of authorized sections to jump to on fault.\n"
> +			"This can be achieved by adding \"%s\" to \n"
> +			"OTHER_TEXT_SECTIONS in scripts/mod/modpost.c.\n",
> +			fromsec, r->r_offset, tosec, tosec, tosec);
> +}
> +
> +static void extable_mismatch_handler(const char* modname, struct elf_info *elf,
> +				     const struct sectioncheck* const mismatch,
> +				     Elf_Rela* r, Elf_Sym* sym,
> +				     const char *fromsec)
> +{
> +	const char* tosec = sec_name(elf, get_secindex(elf, sym));
> +
> +	sec_mismatch_count++;
> +
> +	if (sec_mismatch_verbose)
> +		report_extable_warnings(modname, elf, mismatch, r, sym,
> +					fromsec, tosec);
> +
> +	if (match(tosec, mismatch->bad_tosec))
> +		fatal("The relocation at %s+0x%lx references\n"
> +		      "section \"%s\" which is black-listed.\n"
> +		      "Something is seriously wrong and should be fixed.\n"
> +		      "You might get more information about where this is\n"
> +		      "coming from by using scripts/check_extable.sh %s\n",
> +		      fromsec, 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, 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, r->r_offset, tosec);
> +	}
> +}
> +
>  static void check_section_mismatch(const char *modname, struct elf_info *elf,
>  				   Elf_Rela *r, Elf_Sym *sym, const char *fromsec)
>  {
> @@ -1605,6 +1744,7 @@ static void section_rela(const char *modname, struct elf_info *elf,
>  		/* Skip special sections */
>  		if (is_shndx_special(sym->st_shndx))
>  			continue;
> +		find_extable_entry_size(fromsec, &r, start, rela);
>  		check_section_mismatch(modname, elf, &r, sym, fromsec);
>  	}
>  }
> @@ -1663,6 +1803,7 @@ static void section_rel(const char *modname, struct elf_info *elf,
>  		/* Skip special sections */
>  		if (is_shndx_special(sym->st_shndx))
>  			continue;
> +		find_extable_entry_size(fromsec, &r, start, rel);
>  		check_section_mismatch(modname, elf, &r, sym, fromsec);
>  	}
>  }
> -- 
> 2.0.5
> 

Re: [PATCH 1/7] modpost: add strict white-listing when referencing sections.

[Quentin Casasnovas]

On Tue, Mar 17, 2015 at 09:25:07AM -0700, Linus Torvalds wrote:
> On Tue, Mar 17, 2015 at 5:39 AM, Quentin Casasnovas
> <quentin.casasnovas@oracle.com> wrote:
> > Prints a warning when a section references a section outside a strict
> > white-list.  This will be useful to print a warning if __ex_table
> > references a non-executable section.
> 
> So I think all of these patches are ok,

Cool :)

> but you should probably add a few more people explicitly to the cc.
> 
> Probably Rusty (because modpost) and MIchal (because kbuild). The
> kernel mailing list is so high-volume and untargeted that it's usually
> better as a "archival cc" than as a way to find people to comment and
> merge the patches.
> 

Done. Thanks for the heads up!

Quentin

Re: [PATCH 1/7] modpost: add strict white-listing when referencing sections.

[Rusty Russell]

Quentin Casasnovas <quentin.casasnovas@oracle.com> writes:
> Prints a warning when a section references a section outside a strict
> white-list.  This will be useful to print a warning if __ex_table
> references a non-executable section.

Hi Quentin,

        Really pleasant to read these patches; nice work!

> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index d439856..7094a57 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -925,7 +925,8 @@ enum mismatch {
>  
>  struct sectioncheck {
>  	const char *fromsec[20];
> -	const char *tosec[20];
> +	const char *bad_tosec[20];
> +	const char *good_tosec[20];
>  	enum mismatch mismatch;
>  	const char *symbol_white_list[20];

My only gripe is that these fields are undocumented.  You maintain
the status quo, but some comments indicating what the mean would be
nice.  Perhaps as a separate patch.

In case you need it (for the whole series):
        Acked-by: Rusty Russell <rusty@rustcorp.com.au>

Thanks,
Rusty.

>  };
> @@ -936,19 +937,19 @@ static const struct sectioncheck sectioncheck[] = {
>   */
>  {
>  	.fromsec = { TEXT_SECTIONS, NULL },
> -	.tosec   = { ALL_INIT_SECTIONS, NULL },
> +	.bad_tosec = { ALL_INIT_SECTIONS, NULL },
>  	.mismatch = TEXT_TO_ANY_INIT,
>  	.symbol_white_list = { DEFAULT_SYMBOL_WHITE_LIST, NULL },
>  },
>  {
>  	.fromsec = { DATA_SECTIONS, NULL },
> -	.tosec   = { ALL_XXXINIT_SECTIONS, NULL },
> +	.bad_tosec = { ALL_XXXINIT_SECTIONS, NULL },
>  	.mismatch = DATA_TO_ANY_INIT,
>  	.symbol_white_list = { DEFAULT_SYMBOL_WHITE_LIST, NULL },
>  },
>  {
>  	.fromsec = { DATA_SECTIONS, NULL },
> -	.tosec   = { INIT_SECTIONS, NULL },
> +	.bad_tosec = { INIT_SECTIONS, NULL },
>  	.mismatch = DATA_TO_ANY_INIT,
>  	.symbol_white_list = {
>  		"*_template", "*_timer", "*_sht", "*_ops",
> @@ -957,54 +958,54 @@ static const struct sectioncheck sectioncheck[] = {
>  },
>  {
>  	.fromsec = { TEXT_SECTIONS, NULL },
> -	.tosec   = { ALL_EXIT_SECTIONS, NULL },
> +	.bad_tosec = { ALL_EXIT_SECTIONS, NULL },
>  	.mismatch = TEXT_TO_ANY_EXIT,
>  	.symbol_white_list = { DEFAULT_SYMBOL_WHITE_LIST, NULL },
>  },
>  {
>  	.fromsec = { DATA_SECTIONS, NULL },
> -	.tosec   = { ALL_EXIT_SECTIONS, NULL },
> +	.bad_tosec = { ALL_EXIT_SECTIONS, NULL },
>  	.mismatch = DATA_TO_ANY_EXIT,
>  	.symbol_white_list = { DEFAULT_SYMBOL_WHITE_LIST, NULL },
>  },
>  /* Do not reference init code/data from meminit code/data */
>  {
>  	.fromsec = { ALL_XXXINIT_SECTIONS, NULL },
> -	.tosec   = { INIT_SECTIONS, NULL },
> +	.bad_tosec = { INIT_SECTIONS, NULL },
>  	.mismatch = XXXINIT_TO_SOME_INIT,
>  	.symbol_white_list = { DEFAULT_SYMBOL_WHITE_LIST, NULL },
>  },
>  /* Do not reference exit code/data from memexit code/data */
>  {
>  	.fromsec = { ALL_XXXEXIT_SECTIONS, NULL },
> -	.tosec   = { EXIT_SECTIONS, NULL },
> +	.bad_tosec = { EXIT_SECTIONS, NULL },
>  	.mismatch = XXXEXIT_TO_SOME_EXIT,
>  	.symbol_white_list = { DEFAULT_SYMBOL_WHITE_LIST, NULL },
>  },
>  /* Do not use exit code/data from init code */
>  {
>  	.fromsec = { ALL_INIT_SECTIONS, NULL },
> -	.tosec   = { ALL_EXIT_SECTIONS, NULL },
> +	.bad_tosec = { ALL_EXIT_SECTIONS, NULL },
>  	.mismatch = ANY_INIT_TO_ANY_EXIT,
>  	.symbol_white_list = { DEFAULT_SYMBOL_WHITE_LIST, NULL },
>  },
>  /* Do not use init code/data from exit code */
>  {
>  	.fromsec = { ALL_EXIT_SECTIONS, NULL },
> -	.tosec   = { ALL_INIT_SECTIONS, NULL },
> +	.bad_tosec = { ALL_INIT_SECTIONS, NULL },
>  	.mismatch = ANY_EXIT_TO_ANY_INIT,
>  	.symbol_white_list = { DEFAULT_SYMBOL_WHITE_LIST, NULL },
>  },
>  {
>  	.fromsec = { ALL_PCI_INIT_SECTIONS, NULL },
> -	.tosec   = { INIT_SECTIONS, NULL },
> +	.bad_tosec = { INIT_SECTIONS, NULL },
>  	.mismatch = ANY_INIT_TO_ANY_EXIT,
>  	.symbol_white_list = { NULL },
>  },
>  /* Do not export init/exit functions or data */
>  {
>  	.fromsec = { "__ksymtab*", NULL },
> -	.tosec   = { INIT_SECTIONS, EXIT_SECTIONS, NULL },
> +	.bad_tosec = { INIT_SECTIONS, EXIT_SECTIONS, NULL },
>  	.mismatch = EXPORT_TO_INIT_EXIT,
>  	.symbol_white_list = { DEFAULT_SYMBOL_WHITE_LIST, NULL },
>  }
> @@ -1018,9 +1019,12 @@ static const struct sectioncheck *section_mismatch(
>  	const struct sectioncheck *check = &sectioncheck[0];
>  
>  	for (i = 0; i < elems; i++) {
> -		if (match(fromsec, check->fromsec) &&
> -		    match(tosec, check->tosec))
> -			return check;
> +		if (match(fromsec, check->fromsec)) {
> +			if (check->bad_tosec[0] && match(tosec, check->bad_tosec))
> +				return check;
> +			if (check->good_tosec[0] && !match(tosec, check->good_tosec))
> +				return check;
> +		}
>  		check++;
>  	}
>  	return NULL;
> -- 
> 2.0.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/7] modpost: add strict white-listing when referencing sections.

[Quentin Casasnovas]

[-- Attachment #1: Type: text/plain, Size: 1379 bytes --]

On Fri, Mar 20, 2015 at 11:59:41AM +1030, Rusty Russell wrote:
> Quentin Casasnovas <quentin.casasnovas@oracle.com> writes:
> > Prints a warning when a section references a section outside a strict
> > white-list.  This will be useful to print a warning if __ex_table
> > references a non-executable section.
> 
> Hi Quentin,
> 
>         Really pleasant to read these patches; nice work!
>

Thanks! :)

> > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> > index d439856..7094a57 100644
> > --- a/scripts/mod/modpost.c
> > +++ b/scripts/mod/modpost.c
> > @@ -925,7 +925,8 @@ enum mismatch {
> >  
> >  struct sectioncheck {
> >  	const char *fromsec[20];
> > -	const char *tosec[20];
> > +	const char *bad_tosec[20];
> > +	const char *good_tosec[20];
> >  	enum mismatch mismatch;
> >  	const char *symbol_white_list[20];
> 
> My only gripe is that these fields are undocumented.  You maintain
> the status quo, but some comments indicating what the mean would be
> nice.  Perhaps as a separate patch.
>

Derp, I was sure I had sent a patch following your comment..  Please find
one attached to this e-mail.  It should apply cleanly on top of this
series.

> In case you need it (for the whole series):
>         Acked-by: Rusty Russell <rusty@rustcorp.com.au>
>

Thanks again!

May I aks who's supposed to take the series?  Or maybe it needs more
acking?

Quentin

[-- Attachment #2: 0001-modpost-document-the-use-of-struct-section_check.patch --]
[-- Type: text/x-diff, Size: 1569 bytes --]

>From acab15181879a18140ca3afa69776292e830ea72 Mon Sep 17 00:00:00 2001
From: Quentin Casasnovas <quentin.casasnovas@oracle.com>
Date: Mon, 13 Apr 2015 10:55:38 +0200
Subject: [PATCH] modpost: document the use of struct section_check.

struct section_check is used as a generic way of describing what
relocations are authorized/forbidden when running modpost.  This commit
tries to describe how each field is used.

Signed-off-by: Quentin Casasnovas <quentin.casasnovas@oracle.com>
---
 scripts/mod/modpost.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index dfe9c3c..7b56ae5 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -930,6 +930,26 @@ enum mismatch {
 	EXTABLE_TO_NON_TEXT,
 };
 
+/**
+ * Describe how to match sections on different criterias:
+ *
+ * @fromsec: Array of sections to be matched.
+ *
+ * @bad_tosec: Relocations applied to a section in @fromsec to a section in
+ * this array is forbidden (black-list).  Can be empty.
+ *
+ * @good_tosec: Relocations applied to a section in @fromsec must be
+ * targetting sections in this array (white-list).  Can be empty.
+ *
+ * @mistmatch: Type of mismatch.
+ *
+ * @symbol_white_list: Do not match a relocation to a symbol in this list
+ * even if it is targetting a section in @bad_to_sec.
+ *
+ * @handler: Specific handler to call when a match is found.  If NULL,
+ * default_mismatch_handler() will be called.
+ *
+ */
 struct sectioncheck {
 	const char *fromsec[20];
 	const char *bad_tosec[20];
-- 
2.0.5

Re: [PATCH 7/7] modpost: handle relocations mismatch in __ex_table.

[Rusty Russell]

Quentin Casasnovas <quentin.casasnovas@oracle.com> writes:
> __ex_table is a simple table section where each entry is a pair of
> addresses - the first address is an address which can fault in kernel
> space, and the second address points to where the kernel should jump to
> when handling that fault.  This is how copy_from_user() does not crash the
> kernel if userspace gives a borked pointer for example.

Warnings on 32-bit:

scripts/mod/modpost.c:1562:7: warning: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 4 has type ‘Elf32_Addr’ [-Wformat=]
       to_pretty_name, tosec, tosym_name, to_pretty_name_p);
       ^
scripts/mod/modpost.c:1574:4: warning: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 4 has type ‘Elf32_Addr’ [-Wformat=]
    fromsec, r->r_offset, tosec, tosec, tosec);
    ^
scripts/mod/modpost.c: In function ‘extable_mismatch_handler’:
scripts/mod/modpost.c:1596:9: warning: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 3 has type ‘Elf32_Addr’ [-Wformat=]
         fromsec, r->r_offset, tosec, modname);
         ^
scripts/mod/modpost.c:1604:10: warning: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 3 has type ‘Elf32_Addr’ [-Wformat=]
          fromsec, r->r_offset, tosec);
          ^
scripts/mod/modpost.c:1611:10: warning: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 3 has type ‘Elf32_Addr’ [-Wformat=]
          fromsec, r->r_offset, tosec);
          ^

Fixed like so:

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 7b56ae567fba..b495547e321f 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1557,7 +1557,7 @@ static void report_extable_warnings(const char* modname, struct elf_info* elf,
 
 	warn("%s(%s+0x%lx): Section mismatch in reference"
 	     " from the %s %s%s to the %s %s:%s%s\n",
-	     modname, fromsec, r->r_offset, from_pretty_name,
+	     modname, fromsec, (long)r->r_offset, from_pretty_name,
 	     fromsym_name, from_pretty_name_p,
 	     to_pretty_name, tosec, tosym_name, to_pretty_name_p);
 
@@ -1571,7 +1571,7 @@ static void report_extable_warnings(const char* modname, struct elf_info* elf,
 			"list of authorized sections to jump to on fault.\n"
 			"This can be achieved by adding \"%s\" to \n"
 			"OTHER_TEXT_SECTIONS in scripts/mod/modpost.c.\n",
-			fromsec, r->r_offset, tosec, tosec, tosec);
+			fromsec, (long)r->r_offset, tosec, tosec, tosec);
 }
 
 static void extable_mismatch_handler(const char* modname, struct elf_info *elf,
@@ -1593,7 +1593,7 @@ static void extable_mismatch_handler(const char* modname, struct elf_info *elf,
 		      "Something is seriously wrong and should be fixed.\n"
 		      "You might get more information about where this is\n"
 		      "coming from by using scripts/check_extable.sh %s\n",
-		      fromsec, r->r_offset, tosec, modname);
+		      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"
@@ -1601,14 +1601,14 @@ static void extable_mismatch_handler(const char* modname, struct elf_info *elf,
 			      "it is not possible for the kernel to fault\n"
 			      "at that address.  Something is seriously wrong\n"
 			      "and should be fixed.\n",
-			      fromsec, r->r_offset, tosec);
+			      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, r->r_offset, tosec);
+			      fromsec, (long)r->r_offset, tosec);
 	}
 }
 


Thanks,
Rusty.

Re: [PATCH 1/7] modpost: add strict white-listing when referencing sections.

[Rusty Russell]

Quentin Casasnovas <quentin.casasnovas@oracle.com> writes:
> On Fri, Mar 20, 2015 at 11:59:41AM +1030, Rusty Russell wrote:
>> Quentin Casasnovas <quentin.casasnovas@oracle.com> writes:
>> > Prints a warning when a section references a section outside a strict
>> > white-list.  This will be useful to print a warning if __ex_table
>> > references a non-executable section.
>> 
>> Hi Quentin,
>> 
>>         Really pleasant to read these patches; nice work!
>>
>
> Thanks! :)
>
>> > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
>> > index d439856..7094a57 100644
>> > --- a/scripts/mod/modpost.c
>> > +++ b/scripts/mod/modpost.c
>> > @@ -925,7 +925,8 @@ enum mismatch {
>> >  
>> >  struct sectioncheck {
>> >  	const char *fromsec[20];
>> > -	const char *tosec[20];
>> > +	const char *bad_tosec[20];
>> > +	const char *good_tosec[20];
>> >  	enum mismatch mismatch;
>> >  	const char *symbol_white_list[20];
>> 
>> My only gripe is that these fields are undocumented.  You maintain
>> the status quo, but some comments indicating what the mean would be
>> nice.  Perhaps as a separate patch.
>>
>
> Derp, I was sure I had sent a patch following your comment..  Please find
> one attached to this e-mail.  It should apply cleanly on top of this
> series.
>
>> In case you need it (for the whole series):
>>         Acked-by: Rusty Russell <rusty@rustcorp.com.au>
>>
>
> Thanks again!
>
> May I aks who's supposed to take the series?  Or maybe it needs more
> acking?

If noone else has taken it, I'll do so now.

Applied,
Rusty.

>
> Quentin
>>From acab15181879a18140ca3afa69776292e830ea72 Mon Sep 17 00:00:00 2001
> From: Quentin Casasnovas <quentin.casasnovas@oracle.com>
> Date: Mon, 13 Apr 2015 10:55:38 +0200
> Subject: [PATCH] modpost: document the use of struct section_check.
>
> struct section_check is used as a generic way of describing what
> relocations are authorized/forbidden when running modpost.  This commit
> tries to describe how each field is used.
>
> Signed-off-by: Quentin Casasnovas <quentin.casasnovas@oracle.com>
> ---
>  scripts/mod/modpost.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index dfe9c3c..7b56ae5 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -930,6 +930,26 @@ enum mismatch {
>  	EXTABLE_TO_NON_TEXT,
>  };
>  
> +/**
> + * Describe how to match sections on different criterias:
> + *
> + * @fromsec: Array of sections to be matched.
> + *
> + * @bad_tosec: Relocations applied to a section in @fromsec to a section in
> + * this array is forbidden (black-list).  Can be empty.
> + *
> + * @good_tosec: Relocations applied to a section in @fromsec must be
> + * targetting sections in this array (white-list).  Can be empty.
> + *
> + * @mistmatch: Type of mismatch.
> + *
> + * @symbol_white_list: Do not match a relocation to a symbol in this list
> + * even if it is targetting a section in @bad_to_sec.
> + *
> + * @handler: Specific handler to call when a match is found.  If NULL,
> + * default_mismatch_handler() will be called.
> + *
> + */
>  struct sectioncheck {
>  	const char *fromsec[20];
>  	const char *bad_tosec[20];
> -- 
> 2.0.5

Re: [PATCH 1/7] modpost: add strict white-listing when referencing sections.

[Rusty Russell]

Quentin Casasnovas <quentin.casasnovas@oracle.com> writes:
> + * @good_tosec: Relocations applied to a section in @fromsec must be
> + * targetting sections in this array (white-list).  Can be empty.
> + *
> + * @mistmatch: Type of mismatch.

Spotted a mistake.  I almost mist it.

Cheers,
Rusty.

Re: [PATCH 7/7] modpost: handle relocations mismatch in __ex_table.

[Quentin Casasnovas]

On Mon, Apr 13, 2015 at 08:48:56PM +0930, Rusty Russell wrote:
> Quentin Casasnovas <quentin.casasnovas@oracle.com> writes:
> > __ex_table is a simple table section where each entry is a pair of
> > addresses - the first address is an address which can fault in kernel
> > space, and the second address points to where the kernel should jump to
> > when handling that fault.  This is how copy_from_user() does not crash the
> > kernel if userspace gives a borked pointer for example.
> 
> Warnings on 32-bit:
> 
> [snip/]
> 
> Fixed like so:

Thanks for the fixing, and nice catch on the "mistmatch" ;)
                                                 ^

Quentin

Re: [PATCH 7/7] modpost: handle relocations mismatch in __ex_table.

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 1971 bytes --]

On Tue, Mar 17, 2015 at 01:40:02PM +0100, Quentin Casasnovas wrote:
> __ex_table is a simple table section where each entry is a pair of
> addresses - the first address is an address which can fault in kernel
> space, and the second address points to where the kernel should jump to
> when handling that fault.  This is how copy_from_user() does not crash the
> kernel if userspace gives a borked pointer for example.
> 
> If one of these addresses point to a non-executable section, something is
> seriously wrong since it either means the kernel will never fault from
> there or it will not be able to jump to there.  As both cases are serious
> enough, we simply error out in these cases so the build fails and the
> developper has to fix the issue.
> 
> In case the section is executable, but it isn't referenced in our list of
> authorized sections to point to from __ex_table, we just dump a warning
> giving more information about it.  We do this in case the new section is
> executable but isn't supposed to be executed by the kernel.  This happened
> with .altinstr_replacement, which is executable but is only used to copy
> instructions from - we should never have our instruction pointer pointing
> in .altinstr_replacement.  Admitedly, a proper fix in that case would be to
> just set .altinstr_replacement NX, but we need to warn about future cases
> like this.
> 
> Signed-off-by: Quentin Casasnovas <quentin.casasnovas@oracle.com>
> ---
>  scripts/mod/modpost.c | 141 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 141 insertions(+)

This causes a bunch of mismatch warnings on 32-bit and 64-bit ARM
because there are two additional sections, .text.fixup and
.exception.text that store executable code. I've attached a patch
to fix those, but feel free to squash that into the original commit
if that's still possible.

Also adding Rusty since he applied this to the modules-next tree.

Thierry

[-- Attachment #1.2: 0001-modpost-Whitelist-.text.fixup-and-.exception.text.patch --]
[-- Type: text/x-diff, Size: 1107 bytes --]

From f5199120caafa0056cb18808ffe15af41bb102f3 Mon Sep 17 00:00:00 2001
From: Thierry Reding <treding@nvidia.com>
Date: Tue, 14 Apr 2015 13:59:07 +0200
Subject: [PATCH] modpost: Whitelist .text.fixup and .exception.text

32-bit and 64-bit ARM use these sections to store executable code, so
they must be whitelisted in modpost's table of valid text sections.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 scripts/mod/modpost.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index cbd53e08769d..6a925f200b25 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -876,7 +876,7 @@ static void check_section(const char *modname, struct elf_info *elf,
 #define TEXT_SECTIONS ".text", ".text.unlikely", ".sched.text", \
 		".kprobes.text"
 #define OTHER_TEXT_SECTIONS ".ref.text", ".head.text", ".spinlock.text", \
-		".fixup", ".entry.text"
+		".fixup", ".entry.text", ".text.fixup", ".exception.text"
 
 #define INIT_SECTIONS      ".init.*"
 #define MEM_INIT_SECTIONS  ".meminit.*"
-- 
2.3.5


[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 7/7] modpost: handle relocations mismatch in __ex_table.

[Quentin Casasnovas]

On Tue, Apr 14, 2015 at 02:14:14PM +0200, Thierry Reding wrote:
> On Tue, Mar 17, 2015 at 01:40:02PM +0100, Quentin Casasnovas wrote:
> > If one of these addresses point to a non-executable section, something is
> > seriously wrong since it either means the kernel will never fault from
> > there or it will not be able to jump to there.  As both cases are serious
> > enough, we simply error out in these cases so the build fails and the
> > developper has to fix the issue.
> > 
> > Signed-off-by: Quentin Casasnovas <quentin.casasnovas@oracle.com>
> > ---
> >  scripts/mod/modpost.c | 141 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 141 insertions(+)
> 
> This causes a bunch of mismatch warnings on 32-bit and 64-bit ARM
> because there are two additional sections, .text.fixup and
> .exception.text that store executable code. I've attached a patch
> to fix those, but feel free to squash that into the original commit
> if that's still possible.
>

Thanks Thierry!

Your patch looks good to me, though I was wondering if we should just add
.text.* in the TEXT_SECTIONS macro.  Some architectures define
-ffunction-sections (parisc, score, metag and frv) so there are tons of
useless warnings on these..  It also means the current modpost sanity
checks don't run for those so it might even uncover some real mismatch ;)

Quentin

Re: [PATCH 7/7] modpost: handle relocations mismatch in __ex_table.

[Rusty Russell]

Quentin Casasnovas <quentin.casasnovas@oracle.com> writes:
> On Tue, Apr 14, 2015 at 02:14:14PM +0200, Thierry Reding wrote:
>> On Tue, Mar 17, 2015 at 01:40:02PM +0100, Quentin Casasnovas wrote:
>> > If one of these addresses point to a non-executable section, something is
>> > seriously wrong since it either means the kernel will never fault from
>> > there or it will not be able to jump to there.  As both cases are serious
>> > enough, we simply error out in these cases so the build fails and the
>> > developper has to fix the issue.
>> > 
>> > Signed-off-by: Quentin Casasnovas <quentin.casasnovas@oracle.com>
>> > ---
>> >  scripts/mod/modpost.c | 141 ++++++++++++++++++++++++++++++++++++++++++++++++++
>> >  1 file changed, 141 insertions(+)
>> 
>> This causes a bunch of mismatch warnings on 32-bit and 64-bit ARM
>> because there are two additional sections, .text.fixup and
>> .exception.text that store executable code. I've attached a patch
>> to fix those, but feel free to squash that into the original commit
>> if that's still possible.
>>
>
> Thanks Thierry!
>
> Your patch looks good to me, though I was wondering if we should just add
> .text.* in the TEXT_SECTIONS macro.  Some architectures define
> -ffunction-sections (parisc, score, metag and frv) so there are tons of
> useless warnings on these..  It also means the current modpost sanity
> checks don't run for those so it might even uncover some real mismatch ;)

Yes, but this adds ".exception.text" so a .text.* wildcard won't quite
cover it.

I've applied his patch, then the following:

modpost: handle -ffunction-sections

52dc0595d540 introduced OTHER_TEXT_SECTIONS for identifying what
sections could validly have __ex_table entries.  Unfortunately, it
wasn't tested with -ffunction-sections, which some architectures
use.

Reported-by: kbuild test robot <fengguang.wu@intel.com>
Cc: Quentin Casasnovas <quentin.casasnovas@oracle.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index cbd53e08769d..22dbc604cdb9 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -876,7 +876,7 @@ static void check_section(const char *modname, struct elf_info *elf,
 #define TEXT_SECTIONS ".text", ".text.unlikely", ".sched.text", \
 		".kprobes.text"
 #define OTHER_TEXT_SECTIONS ".ref.text", ".head.text", ".spinlock.text", \
-		".fixup", ".entry.text"
+		".fixup", ".entry.text", ".exception.text", ".text.*"
 
 #define INIT_SECTIONS      ".init.*"
 #define MEM_INIT_SECTIONS  ".meminit.*"

Re: [PATCH 7/7] modpost: handle relocations mismatch in __ex_table.

[Quentin Casasnovas]

On Wed, Apr 15, 2015 at 12:57:37PM +0930, Rusty Russell wrote:
> 
> I've applied his patch, then the following:

Thanks.

> 
> modpost: handle -ffunction-sections
> 
> 52dc0595d540 introduced OTHER_TEXT_SECTIONS for identifying what
> sections could validly have __ex_table entries.  Unfortunately, it
> wasn't tested with -ffunction-sections, which some architectures
> use.
> 
> Reported-by: kbuild test robot <fengguang.wu@intel.com>
> Cc: Quentin Casasnovas <quentin.casasnovas@oracle.com>
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
> 
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index cbd53e08769d..22dbc604cdb9 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -876,7 +876,7 @@ static void check_section(const char *modname, struct elf_info *elf,
>  #define TEXT_SECTIONS ".text", ".text.unlikely", ".sched.text", \
>  		".kprobes.text"
>  #define OTHER_TEXT_SECTIONS ".ref.text", ".head.text", ".spinlock.text", \
> -		".fixup", ".entry.text"
> +		".fixup", ".entry.text", ".exception.text", ".text.*"
>

Is there a reason we're not adding ".text.*" to TEXT_SECTIONS as opposed to
OTHER_TEXT_SECTIONS?  AFAIU, we'll not run the other modpost mismatch
checks when the kernel is compiled with -ffunction-sections otherwise.

I'll send a tentative fix for the divide-by-zero error shortly, sorry about
the mess.

Quentin