[PATCH 4/4] read string sections such as .modinfo with load_strings()

[Andreas Robinson <andr345@xxxxxxxxx>]

load_strings() now avoids buffer overruns by looking for
a string terminator at the end of a section.

Signed-off-by: Andreas Robinson <andr345@xxxxxxxxx>
---
 depmod.c      |   42 ++++++++++++++++++++++--------------------
 elfops.h      |    2 ++
 elfops_core.c |    7 +++++++
 modinfo.c     |   25 +++++++++++++++----------
 modprobe.c    |    9 +++++----
 5 files changed, 51 insertions(+), 34 deletions(-)

diff --git a/depmod.c b/depmod.c
index a8c3a47..2ae8f06 100644
--- a/depmod.c
+++ b/depmod.c
@@ -770,8 +770,8 @@ static void output_aliases(struct module *modules, FILE *out, char *dirname)
 {
 	struct module *i;
 	struct elf_file *file;
-	const char *p;
-	unsigned long size;
+	struct string_table *tbl;
+	int j;
 
 	fprintf(out, "# Aliases extracted from modules themselves.\n");
 	for (i = modules; i; i = i->next) {
@@ -781,19 +781,20 @@ static void output_aliases(struct module *modules, FILE *out, char *dirname)
 		filename2modname(modname, i->pathname);
 
 		/* Grab from old-style .modalias section. */
-		for (p = file->ops->get_aliases(file, &size);
-		     p;
-		     p = next_string(p, &size))
-			fprintf(out, "alias %s %s\n", p, modname);
-
-		/* Grab form new-style .modinfo section. */
-		for (p = file->ops->get_modinfo(file, &size);
-		     p;
-		     p = next_string(p, &size)) {
+		tbl = file->ops->load_strings(file, ".modalias", NULL);
+		for (j = 0; tbl && j < tbl->cnt; j++)
+			fprintf(out, "alias %s %s\n", tbl->str[j], modname);
+		strtbl_free(tbl);
+
+		/* Grab from new-style .modinfo section. */
+		tbl = file->ops->load_strings(file, ".modinfo", NULL);
+		for (j = 0; tbl && j < tbl->cnt; j++) {
+			const char *p = tbl->str[j];
 			if (strstarts(p, "alias="))
 				fprintf(out, "alias %s %s\n",
 					p + strlen("alias="), modname);
 		}
+		strtbl_free(tbl);
 	}
 }
 
@@ -801,9 +802,9 @@ static void output_aliases_bin(struct module *modules, FILE *out, char *dirname)
 {
 	struct module *i;
 	struct elf_file *file;
-	const char *p;
+	struct string_table *tbl;
+	int j;
 	char *alias;
-	unsigned long size;
 	struct index_node *index;
 	int duplicate;
 
@@ -816,10 +817,9 @@ static void output_aliases_bin(struct module *modules, FILE *out, char *dirname)
 		filename2modname(modname, i->pathname);
 
 		/* Grab from old-style .modalias section. */
-		for (p = file->ops->get_aliases(file, &size);
-		     p;
-		     p = next_string(p, &size)) {
-			alias = NOFAIL(strdup(p));
+		tbl = file->ops->load_strings(file, ".modalias", NULL);
+		for (j = 0; tbl && j < tbl->cnt; j++) {
+			alias = NOFAIL(strdup(tbl->str[j]));
 			underscores(alias);
 			duplicate = index_insert(index, alias, modname, i->order);
 			if (duplicate && warn_dups)
@@ -827,11 +827,12 @@ static void output_aliases_bin(struct module *modules, FILE *out, char *dirname)
 					alias, modname);
 			free(alias);
 		}
+		strtbl_free(tbl);
 
 		/* Grab from new-style .modinfo section. */
-		for (p = file->ops->get_modinfo(file, &size);
-		     p;
-		     p = next_string(p, &size)) {
+		tbl = file->ops->load_strings(file, ".modinfo", NULL);
+		for (j = 0; tbl && j < tbl->cnt; j++) {
+			const char *p = tbl->str[j];
 			if (strstarts(p, "alias=")) {
 				alias = NOFAIL(strdup(p + strlen("alias=")));
 				underscores(alias);
@@ -842,6 +843,7 @@ static void output_aliases_bin(struct module *modules, FILE *out, char *dirname)
 				free(alias);
 			}
 		}
+		strtbl_free(tbl);
 	}
 	
 	index_write(index, out);
diff --git a/elfops.h b/elfops.h
index 7069d92..b2714bb 100644
--- a/elfops.h
+++ b/elfops.h
@@ -71,6 +71,8 @@ struct module_ops
 {
 	void *(*load_section)(struct elf_file *module,
 		const char *secname, unsigned long *secsize);
+	struct string_table *(*load_strings)(struct elf_file *module,
+		const char *secname, struct string_table *tbl);
 	struct string_table *(*load_symbols)(struct elf_file *module);
 	struct string_table *(*load_dep_syms)(const char *pathname,
 		struct elf_file *module, struct string_table **types);
diff --git a/elfops_core.c b/elfops_core.c
index 4283d65..a83a362 100644
--- a/elfops_core.c
+++ b/elfops_core.c
@@ -87,6 +87,12 @@ static struct string_table *PERBIT(load_strings)(struct elf_file *module,
 
 	strings = PERBIT(load_section)(module, secname, &size);
 	if (strings) {
+		if (*(strings + size - 1) != '\0') {
+			error("Ignoring malformed section %s in %s ",
+				" - the last string terminator is missing.\n",
+				secname, module->pathname);
+			return NULL;
+		}
 		/* Skip any zero padding. */
 		while (!strings[0]) {
 			strings++;
@@ -338,6 +344,7 @@ static int PERBIT(dump_modversions)(struct elf_file *module)
 
 struct module_ops PERBIT(mod_ops) = {
 	.load_section	= PERBIT(load_section),
+	.load_strings	= PERBIT(load_strings),
 	.load_symbols	= PERBIT(load_symbols),
 	.load_dep_syms	= PERBIT(load_dep_syms),
 	.fetch_tables	= PERBIT(fetch_tables),
diff --git a/modinfo.c b/modinfo.c
index d8412db..64f1139 100644
--- a/modinfo.c
+++ b/modinfo.c
@@ -50,9 +50,10 @@ static struct param *add_param(const char *name, struct param **list)
 	return i;
 }
 
-static void print_tag(const char *tag, const char *info, unsigned long size,
+static void print_tag(const char *tag, struct string_table *tags,
 		      const char *filename, char sep)
 {
+	int j;
 	unsigned int taglen = strlen(tag);
 
 	if (streq(tag, "filename")) {
@@ -60,18 +61,22 @@ static void print_tag(const char *tag, const char *info, unsigned long size,
 		return;
 	}
 
-	for (; info; info = next_string(info, &size))
+	for (j = 0; j < tags->cnt; j++) {
+		const char *info = tags->str[j];
 		if (strncmp(info, tag, taglen) == 0 && info[taglen] == '=')
 			printf("%s%c", info + taglen + 1, sep);
+	}
 }
 
-static void print_all(const char *info, unsigned long size,
+static void print_all(struct string_table *tags,
 		      const char *filename, char sep)
 {
+	int j;
 	struct param *i, *params = NULL;
 
 	printf("%-16s%s%c", "filename:", filename, sep);
-	for (; info; info = next_string(info, &size)) {
+	for (j = 0; j < tags->cnt; j++) {
+		const char *info = tags->str[j];
 		char *eq, *colon;
 
 		/* We expect this in parm and parmtype. */
@@ -294,8 +299,7 @@ int main(int argc, char *argv[])
 	}
 
 	for (opt = optind; opt < argc; opt++) {
-		void *info;
-		unsigned long infosize = 0;
+		struct string_table *tags;
 		struct elf_file *mod;
 
 		mod = grab_module(argv[opt], kernel, basedir);
@@ -303,15 +307,16 @@ int main(int argc, char *argv[])
 			ret = 1;
 			continue;
 		}
-		info = mod->ops->get_modinfo(mod, &infosize);
-		if (!info) {
+		tags = mod->ops->load_strings(mod, ".modinfo", NULL);
+		if (!tags) {
 			release_elf_file(mod);
 			continue;
 		}
 		if (field)
-			print_tag(field, info, infosize, mod->pathname, sep);
+			print_tag(field, tags, mod->pathname, sep);
 		else
-			print_all(info, infosize, mod->pathname, sep);
+			print_all(tags, mod->pathname, sep);
+		strtbl_free(tags);
 		release_elf_file(mod);
 	}
 	return ret;
diff --git a/modprobe.c b/modprobe.c
index c6ef155..4cb7397 100644
--- a/modprobe.c
+++ b/modprobe.c
@@ -326,15 +326,16 @@ static void rename_module(struct elf_file *module,
 
 static void clear_magic(struct elf_file *module)
 {
-	const char *p;
-	unsigned long len;
+	struct string_table *tbl;
+	int j;
 
 	/* Old-style: __vermagic section */
 	module->ops->strip_section(module, "__vermagic");
 
 	/* New-style: in .modinfo section */
-	p = module->ops->get_modinfo(module, &len);
-	for (; p; p = next_string(p, &len)) {
+	tbl = module->ops->load_strings(module, ".modinfo", NULL);
+	for (j = 0; tbl && j < tbl->cnt; j++) {
+		const char *p = tbl->str[j];
 		if (strstarts(p, "vermagic=")) {
 			memset((char *)p, 0, strlen(p));
 			return;
-- 
1.6.0.4


--
To unsubscribe from this list: send the line "unsubscribe linux-modules" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html