On Thu, 2009-05-21 at 19:19 +0100, Alan Jenkins wrote:
> On 5/21/09, Andreas Robinson <andr345@xxxxxxxxx> wrote:
> > On Thu, 2009-05-21 at 12:33 +0100, Alan Jenkins wrote:
> >> On 5/21/09, Andreas Robinson <andr345@xxxxxxxxx> wrote:
> >> >
> >> > error("Could not read '%s': %s\n",
> >> > mod->filename, strerror(errno));
> >>
> >>
> >> This will change the error message to something less helpful. You
> >> should probably use insert_moderror() instead of strerror().
> >
> > I'm not so sure about that. strerror(ENOEXEC) == "Exec format error",
> > and while I agree it is less clear than "invalid module format", it is
> > exactly the right message for this kind of error. I.e we're trying to
> > load an executable but it really isn't => Exec format error. It is
> > simply seen less often than say, "File not found".
>
> Yeah, I didn't say it was actually wrong.
>
> > To use insert_moderror(), the message would have to be formatted like
> > this:
> >
> > error("Could not read '%s': %s\n", mod->filename,
> > (errno == ENOEXEC) ? insert_moderror(errno) : strerror(errno));
> >
> > Though this is probably better:
> >
> > error("Could not read '%s': %s\n", mod->filename,
> > (errno == ENOEXEC) ? "invalid module format" : strerror(errno));
> >
> > since insert_moderror() is written explicitly for init_module().
> >
> > If it isn't done like that, we would get insert_moderror(ENOENT) ==
> > "Unknown symbol in module, or unknown parameter (see dmesg)" which
> > clearly isn't the "File not found" we're expecting.
>
> Agreed, that would be very wrong. The second version above looks good.
And, here's the reworked version.
>From d1877dc6e4bbc5344990cc6df6a35cc45f246788 Mon Sep 17 00:00:00 2001
From: Andreas Robinson <andr345@xxxxxxxxx>
Date: Thu, 21 May 2009 22:29:52 +0200
Subject: [PATCH] modprobe, tests: make modprobe fail noisily on non-ELF files
Also remove fake placeholder modules in the test suite
e.g "echo Test > foo.ko && modprobe foo".
They don't work anymore.
Signed-off-by: Andreas Robinson <andr345@xxxxxxxxx>
---
modprobe.c | 21 +++------------------
tests/test-modprobe-indexed/10alias.sh | 9 +++++----
tests/test-modprobe/10alias.sh | 9 +++++----
tests/test-modprobe/26blacklist.sh | 7 ++++---
4 files changed, 17 insertions(+), 29 deletions(-)
diff --git a/modprobe.c b/modprobe.c
index a3cdbf6..f84663f 100644
--- a/modprobe.c
+++ b/modprobe.c
@@ -666,23 +666,9 @@ static int insmod(struct list_head *list,
module = grab_elf_file_fd(mod->filename, fd);
if (!module) {
- /* This is an ugly hack that maintains the logic where
- * init_module() sets errno = ENOEXEC if the file is
- * not an ELF object.
- */
- if (errno == ENOEXEC) {
- struct stat st;
- optstring = add_extra_options(mod->modname,
- optstring, options);
- if (dry_run)
- goto out;
- fstat(fd, &st);
- ret = init_module(NULL, st.st_size, optstring);
- goto out_hack;
- }
-
- error("Could not read '%s': %s\n",
- mod->filename, strerror(errno));
+ error("Could not read '%s': %s\n", mod->filename,
+ (errno == ENOEXEC) ? "Invalid module format" :
+ strerror(errno));
goto out_unlock;
}
if (newname)
@@ -701,7 +687,6 @@ static int insmod(struct list_head *list,
goto out;
ret = init_module(module->data, module->len, optstring);
-out_hack:
if (ret != 0) {
if (errno == EEXIST) {
if (first_time)
diff --git a/tests/test-modprobe-indexed/10alias.sh b/tests/test-modprobe-indexed/10alias.sh
index bd16131..c2a2f5a 100644
--- a/tests/test-modprobe-indexed/10alias.sh
+++ b/tests/test-modprobe-indexed/10alias.sh
@@ -21,7 +21,8 @@ rm -f $MODULE_DIR/modules.alias
rm -f $MODULE_DIR/modules.alias.bin
rm -f tests/tmp/etc/modprobe.d/modprobe.conf
-echo Test > $MODULE_DIR/kernel/foo.ko
+cp tests/data/$BITNESS/complex/complex_a-$BITNESS.ko $MODULE_DIR/kernel/foo.ko
+SIZE2=`wc -c < $MODULE_DIR/kernel/foo.ko`
# Shouldn't complain if can't open modules.alias
[ "`modprobe bar 2>&1`" = "FATAL: Module bar not found." ]
@@ -34,12 +35,12 @@ modindex -o $MODULE_DIR/modules.alias.bin < $MODULE_DIR/modules.alias.bin.temp
# Normal alias should override it.
mkdir -p tests/tmp/etc/modprobe.d
echo 'alias bar foo' > tests/tmp/etc/modprobe.d/modprobe.conf
-[ "`modprobe foo 2>&1`" = "INIT_MODULE: 5 " ]
+[ "`modprobe foo 2>&1`" = "INIT_MODULE: $SIZE2 " ]
# If there's a real module, alias from modules.alias must NOT override.
echo "foo alias_$BITNESS" > $MODULE_DIR/modules.alias.bin.temp
modindex -o $MODULE_DIR/modules.alias.bin < $MODULE_DIR/modules.alias.bin.temp
-[ "`modprobe foo 2>&1`" = "INIT_MODULE: 5 " ]
+[ "`modprobe foo 2>&1`" = "INIT_MODULE: $SIZE2 " ]
# If there's an install command, modules.alias must not override.
echo 'install bar echo foo' > tests/tmp/etc/modprobe.d/modprobe.conf
@@ -60,5 +61,5 @@ modindex -o $MODULE_DIR/modules.alias.bin < $MODULE_DIR/modules.alias.bin.temp
OUT="`modprobe bar 2>&1`"
[ "$OUT" = "INIT_MODULE: $SIZE option2 option1
-INIT_MODULE: 5 option1" ] || [ "$OUT" = "INIT_MODULE: 5 option1
+INIT_MODULE: $SIZE2 option1" ] || [ "$OUT" = "INIT_MODULE: $SIZE2 option1
INIT_MODULE: $SIZE option2 option1" ]
diff --git a/tests/test-modprobe/10alias.sh b/tests/test-modprobe/10alias.sh
index ebe2ea0..c5abf43 100755
--- a/tests/test-modprobe/10alias.sh
+++ b/tests/test-modprobe/10alias.sh
@@ -16,7 +16,8 @@ SIZE=`wc -c < tests/data/$BITNESS/alias/alias-$BITNESS.ko`
echo "/lib/modules/$MODTEST_UNAME/kernel/alias-$BITNESS.ko:" > $MODULE_DIR/modules.dep
echo "/lib/modules/$MODTEST_UNAME/kernel/foo.ko:" >> $MODULE_DIR/modules.dep
-echo Test > $MODULE_DIR/kernel/foo.ko
+cp tests/data/$BITNESS/complex/complex_a-$BITNESS.ko $MODULE_DIR/kernel/foo.ko
+SIZE2=`wc -c < $MODULE_DIR/kernel/foo.ko`
# Shouldn't complain if can't open modules.alias
[ "`modprobe bar 2>&1`" = "FATAL: Module bar not found." ]
@@ -28,11 +29,11 @@ echo "alias bar alias-$BITNESS" > $MODULE_DIR/modules.alias
# Normal alias should override it.
mkdir -p tests/tmp/etc/modprobe.d
echo 'alias bar foo' > tests/tmp/etc/modprobe.d/modprobe.conf
-[ "`modprobe foo 2>&1`" = "INIT_MODULE: 5 " ]
+[ "`modprobe foo 2>&1`" = "INIT_MODULE: $SIZE2 " ]
# If there's a real module, alias from modules.alias must NOT override.
echo "alias foo alias-$BITNESS" > $MODULE_DIR/modules.alias
-[ "`modprobe foo 2>&1`" = "INIT_MODULE: 5 " ]
+[ "`modprobe foo 2>&1`" = "INIT_MODULE: $SIZE2 " ]
# If there's an install command, modules.alias must not override.
echo 'install bar echo foo' > tests/tmp/etc/modprobe.d/modprobe.conf
@@ -51,5 +52,5 @@ echo "alias bar foo" >> $MODULE_DIR/modules.alias
OUT="`modprobe bar 2>&1`"
[ "$OUT" = "INIT_MODULE: $SIZE option2 option1
-INIT_MODULE: 5 option1" ] || [ "$OUT" = "INIT_MODULE: 5 option1
+INIT_MODULE: $SIZE2 option1" ] || [ "$OUT" = "INIT_MODULE: $SIZE2 option1
INIT_MODULE: $SIZE option2 option1" ]
diff --git a/tests/test-modprobe/26blacklist.sh b/tests/test-modprobe/26blacklist.sh
index 8fcff39..482ee93 100755
--- a/tests/test-modprobe/26blacklist.sh
+++ b/tests/test-modprobe/26blacklist.sh
@@ -15,7 +15,8 @@ SIZE=`wc -c < tests/data/$BITNESS/alias/alias-$BITNESS.ko`
echo "/lib/modules/$MODTEST_UNAME/kernel/alias-$BITNESS.ko:" > $MODULE_DIR/modules.dep
echo "/lib/modules/$MODTEST_UNAME/kernel/foo.ko:" >> $MODULE_DIR/modules.dep
-echo Test > $MODULE_DIR/kernel/foo.ko
+cp tests/data/$BITNESS/complex/complex_a-$BITNESS.ko $MODULE_DIR/kernel/foo.ko
+SIZE2=`wc -c < $MODULE_DIR/kernel/foo.ko`
# First, alias found in modules.alias works.
echo "alias bar alias-$BITNESS" > $MODULE_DIR/modules.alias
@@ -28,7 +29,7 @@ echo "blacklist alias-$BITNESS" > tests/tmp/etc/modprobe.d/modprobe.conf
# Blacklist doesn't effect other aliases.
echo "alias bar foo" >> $MODULE_DIR/modules.alias
-[ "`modprobe bar 2>&1`" = "INIT_MODULE: 5 " ]
+[ "`modprobe bar 2>&1`" = "INIT_MODULE: $SIZE2 " ]
# Blacklist both.
echo "blacklist foo" >> tests/tmp/etc/modprobe.d/modprobe.conf
@@ -38,5 +39,5 @@ echo "blacklist foo" >> tests/tmp/etc/modprobe.d/modprobe.conf
rm -f tests/tmp/etc/modprobe.d/modprobe.conf
RESULT="`modprobe bar 2>&1`"
[ "$RESULT" = "INIT_MODULE: $SIZE
-INIT_MODULE: 5 " ] || [ "$RESULT" = "INIT_MODULE: 5
+INIT_MODULE: $SIZE2 " ] || [ "$RESULT" = "INIT_MODULE: $SIZE2
INIT_MODULE: $SIZE " ]
--
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
