Skip to contentSkip to conversation

module: return error when mod_sysfs_init() failed

PATCH Applied Akinobu Mita 3 participants 5 messages Sep 29, 2007

lore
Share & tools
lore

Akinobu Mita

author
#0 lore raw

PATCH 1/1 module: return error when mod_sysfs_init() failed

load_module() returns zero when mod_sysfs_init() fails, then the module loading will succeed accidentally.

This patch makes load_module() return error correctly in that case.

Cc Greg Kroah-Hartman <[email protected]>
Cc Rusty Russell <[email protected]>
Signed-off-by Akinobu Mita <[email protected]>
---
 kernel/module.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
kernel/module.c +2 -1
Index: 2.6-git/kernel/module.c
===================================================================
@@ -1782,7 +1782,8 @@ static struct module *load_module(void _
 	module_unload_init(mod);
 
 	/* Initialize kobject, so we can reference it. */
-	if (mod_sysfs_init(mod) != 0)
+	err = mod_sysfs_init(mod);
+	if (err)
 		goto cleanup;
 
 	/* Set up license info based on the info section */
Applied
#1 lore raw
Quoted 4 lines · Akinobu Mita

load_module() returns zero when mod_sysfs_init() fails, then the module loading will succeed accidentally.

This patch makes load_module() return error correctly in that case.

Thanks, applied.

Cheers,
Rusty.

#2 lore raw
In reply to Akinobu Mita
Quoted 18 lines · Akinobu Mita

load_module() returns zero when mod_sysfs_init() fails, then the module loading will succeed accidentally.

This patch makes load_module() return error correctly in that case.

Cc Greg Kroah-Hartman <[email protected]>
Cc Rusty Russell <[email protected]>
Signed-off-by Akinobu Mita <[email protected]>
---
 kernel/module.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Index: 2.6-git/kernel/module.c
===================================================================

kernel/module.c static struct module *load_module(void _ · line 1787

Quoted 7 lines
kernel/module.c +2 -1
@@ -1782,7 +1782,8 @@ static struct module *load_module(void _
 	module_unload_init(mod);
 
 	/* Initialize kobject, so we can reference it. */
-	if (mod_sysfs_init(mod) != 0)
+	err = mod_sysfs_init(mod);
+	if (err)
 		goto cleanup;

I must be still asleep this morning, but I think this patch does the exact same thing as the original code does, right? Otherwise, this code would always be failing.

Or do I just need to go get my morning coffee to wake up and see the problem here?

thanks,

greg k-h

author
#3 lore raw

2007/9/29, Greg KH <[email protected]>:

Quoted 17 lines

kernel/module.c static struct module *load_module(void _ · line 1787

Quoted 12 lines

Index: 2.6-git/kernel/module.c
===================================================================

kernel/module.c +2 -1
@@ -1782,7 +1782,8 @@ static struct module *load_module(void _
      module_unload_init(mod);
 
      /* Initialize kobject, so we can reference it. */
-     if (mod_sysfs_init(mod) != 0)
+     err = mod_sysfs_init(mod);
+     if (err)
              goto cleanup;

I must be still asleep this morning, but I think this patch does the exact same thing as the original code does, right? Otherwise, this code would always be failing.

Quoted 2 lines

Or do I just need to go get my morning coffee to wake up and see the problem here?

Hello,

In the original code, the "err" is zero before goto cleanup. This "err" will be the return value of load_module().

load_module() is the function which returns error as pointer and the expression IS_ERR(NULL) is false. So the caller of load_module() cannot catch that error.

I found this problem when I was running the fault injection test script in Documentation/fault-injection/fault-injection.txt with random module.

#!/bin/bash

FAILTYPE=failslab
echo Y > /debug/$FAILTYPE/task-filter
echo 10 > /debug/$FAILTYPE/probability
echo 100 > /debug/$FAILTYPE/interval
echo -1 > /debug/$FAILTYPE/times
echo 0 > /debug/$FAILTYPE/space
echo 2 > /debug/$FAILTYPE/verbose
echo 0 > /debug/$FAILTYPE/ignore-gfp-wait

faulty_system()

{
        bash -c "echo 1 > /proc/self/make-it-fail && exec $*"
}

if [ $# -eq 0 ]
then
echo "Usage: $0 modulename [ modulename ... ]" exit 1
fi

for m in $*
do
echo inserting $m...
faulty_system modprobe $m

echo removing $m...
faulty_system modprobe -r $m
done

#4 lore raw
Quoted 18 lines · Akinobu Mita

2007/9/29, Greg KH <[email protected]>:

Quoted 17 lines

kernel/module.c static struct module *load_module(void _ · line 1787

Quoted 12 lines

Index: 2.6-git/kernel/module.c
===================================================================

kernel/module.c +2 -1
@@ -1782,7 +1782,8 @@ static struct module *load_module(void _
      module_unload_init(mod);
 
      /* Initialize kobject, so we can reference it. */
-     if (mod_sysfs_init(mod) != 0)
+     err = mod_sysfs_init(mod);
+     if (err)
              goto cleanup;

I must be still asleep this morning, but I think this patch does the exact same thing as the original code does, right? Otherwise, this code would always be failing.

Quoted 7 lines
Quoted 2 lines

Or do I just need to go get my morning coffee to wake up and see the problem here?

Hello,

In the original code, the "err" is zero before goto cleanup. This "err" will be the return value of load_module().

Ah, ok, that makes sense now, thanks. It was the error not getting returned, it was not the fact that we were incorrectly checking the return value of mod_sysfs_init.

thanks for clearing this up.

greg k-h