linux-kernel - [patch 2.6.20-git] remove modpost false warnings on ARM

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 15 Feb 2007 19:10:45 -0800
From:	David Brownell <david-b@...bell.net>
To:	Linux Kernel list <linux-kernel@...r.kernel.org>
Cc:	Andi Kleen <ak@...e.de>, Andrew Morton <akpm@...ux-foundation.org>,
	ebiederm@...ssion.com, kai@...maschewski.name,
	Russell King <rmk@....linux.org.uk>, sam@...nborg.org,
	rusty@...tcorp.com.au
Subject: [patch 2.6.20-git] remove modpost false warnings on ARM

This patch stops "modpost" from issuing erroneous modpost warnings on ARM
builds, which it's been doing since since maybe last summer.  A canonical
example would be driver method table entries:

  WARNING: <path> - Section mismatch: reference to .exit.text:<name>_remove
	from .data after '$d' (at offset 0x4)

That "$d" symbol is generated by tools conformant with ARM ABI specs; in
this case it's a symbol **in the middle of** a "<name>_driver" struct.

The erroneous warnings appear to be issued because "modpost" whitelists
references from "<name>_driver" data into init and exit sections ... but
doesn't know should also include those "$d" mapping symbols, which are
not otherwise associated with "<name>_driver" symbols.

This patch prevents the modpost symbol lookup code from ever returning
those mapping symbols, so it will return a whitelisted symbol instead.
Then things work as expected.

Now to revert various code-bloating "fixes" that got merged because of
this modpost bug....

Signed-off-by: David Brownell <dbrownell@...rs.sourceforge.net>
---
NOTE:  this is a **RESEND** of a patch against 2.6.20-rc4 ... it's
past time to merge a fix, please!!  I don't see an entry for "modpost"
in the MAINTAINERS file, so I'm trying to CC anyone who "git" says
has been involved recently...

However, it also seems to me that these modpost checks are wrong:

  * Lingering pointers that point into sections modprobe removes are
    *always* unsafe ... including probe() methods marked "__init"
    on hotpluggable busses.  Trivial fix:  use __devinit instead;
    or maybe platform_driver_probe().

  * Lingering pointers that point into sections that aren't removed
    are *never* unsafe ... including this remove() method case, since
    module unloading is configured and the __exit stuff must stay.

Whitelisting the former means not reporting potential oopsing cases;
dangerous.  Whereas even *checking* the latter is a waste of effort.

Index: at91/scripts/mod/modpost.c
===================================================================
--- at91.orig/scripts/mod/modpost.c	2007-02-15 18:20:15.000000000 -0800
+++ at91/scripts/mod/modpost.c	2007-02-15 18:22:50.000000000 -0800
@@ -686,6 +686,30 @@ static Elf_Sym *find_elf_symbol(struct e
 	return NULL;
 }

+static inline int is_arm_mapping_symbol(const char *str)
+{
+	return str[0] == '$' && strchr("atd", str[1])
+	       && (str[2] == '\0' || str[2] == '.');
+}
+
+/*
+ * If there's no name there, ignore it; likewise, ignore it if it's
+ * one of the magic symbols emitted used by current ARM tools.
+ *
+ * Otherwise if find_symbols_between() returns those symbols, they'll
+ * fail the whitelist tests and cause lots of false alarms ... fixable
+ * only by merging __exit and __init sections into __text, bloating
+ * the kernel (which is especially evil on embedded platforms).
+ */
+static inline int is_valid_name(struct elf_info *elf, Elf_Sym *sym)
+{
+	const char *name = elf->strtab + sym->st_name;
+
+	if (!name || !strlen(name))
+		return 0;
+	return !is_arm_mapping_symbol(name);
+}
+
 /*
  * Find symbols before or equal addr and after addr - in the section sec.
  * If we find two symbols with equal offset prefer one with a valid name.
@@ -714,16 +738,15 @@ static void find_symbols_between(struct 
 		symsec = secstrings + elf->sechdrs[sym->st_shndx].sh_name;
 		if (strcmp(symsec, sec) != 0)
 			continue;
+		if (!is_valid_name(elf, sym))
+			continue;
 		if (sym->st_value <= addr) {
 			if ((addr - sym->st_value) < beforediff) {
 				beforediff = addr - sym->st_value;
 				*before = sym;
 			}
 			else if ((addr - sym->st_value) == beforediff) {
-				/* equal offset, valid name? */
-				const char *name = elf->strtab + sym->st_name;
-				if (name && strlen(name))
-					*before = sym;
+				*before = sym;
 			}
 		}
 		else
@@ -733,10 +756,7 @@ static void find_symbols_between(struct 
 				*after = sym;
 			}
 			else if ((sym->st_value - addr) == afterdiff) {
-				/* equal offset, valid name? */
-				const char *name = elf->strtab + sym->st_name;
-				if (name && strlen(name))
-					*after = sym;
+				*after = sym;
 			}
 		}
 	}
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/