[git commit] modprobe-small: fix bogus handling of unpack errors

Denys Vlasenko vda.linux at googlemail.com
Mon Jun 20 09:02:40 UTC 2016


commit: https://git.busybox.net/busybox/commit/?id=5c3e060604444b18303f55e631637c28d889704f
branch: https://git.busybox.net/busybox/commit/?id=refs/heads/master

"modprobe minix; echo $?"

Was:
modprobe: corrupted data
modprobe: read error from 'kernel/fs/minix/minix.ko.xz': No such file or directory
modprobe: corrupted data
modprobe: read error from 'kernel/fs/minix/minix.ko.xz': No such file or directory
modprobe: corrupted data
modprobe: read error from 'kernel/fs/minix/minix.ko.xz'
modprobe: 'kernel/fs/minix/minix.ko.xz': Success
0

Now:
modprobe: corrupted data
modprobe: read error from 'kernel/fs/minix/minix.ko.xz'
1

Signed-off-by: Denys Vlasenko <vda.linux at googlemail.com>
---
 modutils/modprobe-small.c | 40 ++++++++++++++++++++++++++++------------
 1 file changed, 28 insertions(+), 12 deletions(-)

diff --git a/modutils/modprobe-small.c b/modutils/modprobe-small.c
index ffb46e6..5936e48 100644
--- a/modutils/modprobe-small.c
+++ b/modutils/modprobe-small.c
@@ -45,6 +45,7 @@ typedef struct module_info {
 	char *pathname;
 	char *aliases;
 	char *deps;
+	smallint open_read_failed;
 } module_info;
 
 /*
@@ -222,7 +223,8 @@ static int load_module(const char *fname, const char *options)
 #endif
 }
 
-static void parse_module(module_info *info, const char *pathname)
+/* Returns !0 if open/read was unsuccessful */
+static int parse_module(module_info *info, const char *pathname)
 {
 	char *module_image;
 	char *ptr;
@@ -231,6 +233,7 @@ static void parse_module(module_info *info, const char *pathname)
 	dbg1_error_msg("parse_module('%s')", pathname);
 
 	/* Read (possibly compressed) module */
+	errno = 0;
 	len = 64 * 1024 * 1024; /* 64 Mb at most */
 	module_image = xmalloc_open_zipped_read_close(pathname, &len);
 	/* module_image == NULL is ok here, find_keyword handles it */
@@ -298,9 +301,11 @@ static void parse_module(module_info *info, const char *pathname)
 		dbg2_error_msg("dep:'%s'", ptr);
 		append(ptr);
 	}
+	free(module_image);
 	info->deps = copy_stringbuf();
 
-	free(module_image);
+	info->open_read_failed = (module_image == NULL);
+	return info->open_read_failed;
 }
 
 static FAST_FUNC int fileAction(const char *pathname,
@@ -331,7 +336,8 @@ static FAST_FUNC int fileAction(const char *pathname,
 
 	dbg1_error_msg("'%s' module name matches", pathname);
 	module_found_idx = cur;
-	parse_module(&modinfo[cur], pathname);
+	if (parse_module(&modinfo[cur], pathname) != 0)
+		return TRUE; /* failed to open/read it, no point in trying loading */
 
 	if (!(option_mask32 & OPT_r)) {
 		if (load_module(pathname, module_load_options) == 0) {
@@ -608,13 +614,14 @@ static int rmmod(const char *filename)
 #define process_module(a,b) process_module(a)
 #define cmdline_options ""
 #endif
-static void process_module(char *name, const char *cmdline_options)
+static int process_module(char *name, const char *cmdline_options)
 {
 	char *s, *deps, *options;
 	module_info **infovec;
 	module_info *info;
 	int infoidx;
 	int is_remove = (option_mask32 & OPT_r) != 0;
+	int exitcode = EXIT_SUCCESS;
 
 	dbg1_error_msg("process_module('%s','%s')", name, cmdline_options);
 
@@ -628,7 +635,7 @@ static void process_module(char *name, const char *cmdline_options)
 		 * (compat note: this allows and strips .ko suffix)
 		 */
 		rmmod(name);
-		return;
+		return EXIT_SUCCESS;
 	}
 
 	/*
@@ -639,7 +646,7 @@ static void process_module(char *name, const char *cmdline_options)
 	 */
 	if (!is_remove && already_loaded(name)) {
 		dbg1_error_msg("nothing to do for '%s'", name);
-		return;
+		return EXIT_SUCCESS;
 	}
 
 	options = NULL;
@@ -741,6 +748,11 @@ static void process_module(char *name, const char *cmdline_options)
 			dbg1_error_msg("'%s': blacklisted", info->pathname);
 			continue;
 		}
+		if (info->open_read_failed) {
+			/* We already tried it, didn't work. Don't try load again */
+			exitcode = EXIT_FAILURE;
+			continue;
+		}
 		errno = 0;
 		if (load_module(info->pathname, options) != 0) {
 			if (EEXIST != errno) {
@@ -752,13 +764,14 @@ static void process_module(char *name, const char *cmdline_options)
 					info->pathname,
 					moderror(errno));
 			}
+			exitcode = EXIT_FAILURE;
 		}
 	}
  ret:
 	free(infovec);
 	free(options);
-//TODO: return load attempt result from process_module.
-//If dep didn't load ok, continuing makes little sense.
+
+	return exitcode;
 }
 #undef cmdline_options
 
@@ -865,6 +878,7 @@ The following options are useful for people managing distributions:
 int modprobe_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
 int modprobe_main(int argc UNUSED_PARAM, char **argv)
 {
+	int exitcode;
 	struct utsname uts;
 	char applet0 = applet_name[0];
 	IF_FEATURE_MODPROBE_SMALL_OPTIONS_ON_CMDLINE(char *options;)
@@ -970,21 +984,23 @@ int modprobe_main(int argc UNUSED_PARAM, char **argv)
 			bb_error_msg_and_die("can't insert '%s': %s",
 					*argv, moderror(errno));
 		}
-		return 0;
+		return EXIT_SUCCESS;
 	}
 
 	/* Try to load modprobe.dep.bb */
-	if ('r' != applet0) /* not rmmod */
+	if ('r' != applet0) { /* not rmmod */
 		load_dep_bb();
+	}
 
 	/* Load/remove modules.
 	 * Only rmmod/modprobe -r loops here, insmod/modprobe has only argv[0] */
+	exitcode = EXIT_SUCCESS;
 	do {
-		process_module(*argv, options);
+		exitcode |= process_module(*argv, options);
 	} while (*++argv);
 
 	if (ENABLE_FEATURE_CLEAN_UP) {
 		IF_FEATURE_MODPROBE_SMALL_OPTIONS_ON_CMDLINE(free(options);)
 	}
-	return EXIT_SUCCESS;
+	return exitcode;
 }


More information about the busybox-cvs mailing list