Re: [PATCH] module: return error when mod_sysfs_init() failed
On Sun, Sep 30, 2007 at 12:37:10AM +0900, Akinobu Mita wrote: > 2007/9/29, Greg KH <[EMAIL PROTECTED]>: > > > Index: 2.6-git/kernel/module.c > > > === > > > --- 2.6-git.orig/kernel/module.c > > > +++ 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; > > > > 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? > > 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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] module: return error when mod_sysfs_init() failed
2007/9/29, Greg KH <[EMAIL PROTECTED]>:
> > Index: 2.6-git/kernel/module.c
> > ===
> > --- 2.6-git.orig/kernel/module.c
> > +++ 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;
>
> 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?
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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] module: return error when mod_sysfs_init() failed
On Sat, Sep 29, 2007 at 07:06:53PM +0900, Akinobu Mita wrote: > 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 > === > --- 2.6-git.orig/kernel/module.c > +++ 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; 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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] module: return error when mod_sysfs_init() failed
On Sat, 2007-09-29 at 19:06 +0900, Akinobu Mita wrote: > 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. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] 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(-) Index: 2.6-git/kernel/module.c === --- 2.6-git.orig/kernel/module.c +++ 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 */ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] 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(-) Index: 2.6-git/kernel/module.c === --- 2.6-git.orig/kernel/module.c +++ 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 */ - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] module: return error when mod_sysfs_init() failed
On Sat, 2007-09-29 at 19:06 +0900, Akinobu Mita wrote: 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. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] module: return error when mod_sysfs_init() failed
On Sat, Sep 29, 2007 at 07:06:53PM +0900, Akinobu Mita wrote: 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 === --- 2.6-git.orig/kernel/module.c +++ 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; 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 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] module: return error when mod_sysfs_init() failed
2007/9/29, Greg KH [EMAIL PROTECTED]:
Index: 2.6-git/kernel/module.c
===
--- 2.6-git.orig/kernel/module.c
+++ 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;
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?
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
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] module: return error when mod_sysfs_init() failed
On Sun, Sep 30, 2007 at 12:37:10AM +0900, Akinobu Mita wrote: 2007/9/29, Greg KH [EMAIL PROTECTED]: Index: 2.6-git/kernel/module.c === --- 2.6-git.orig/kernel/module.c +++ 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; 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? 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 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
