On Wed, Sep 12, 2012 at 08:06:16AM -0700, Kees Cook wrote: > Caught by smatch: > kernel/module.c:2450 copy_module_from_user() warn: maybe return -EFAULT instead of the bytes remaining? > > Clean up the copy_from_user() call to not report a positive value. > With this patch, init_module() will report errors from copy_from_user > (before it would always only report -EFAULT when err != 0). > > Reported-by: Fengguang Wu <fengguang.wu@xxxxxxxxx> > Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx> > --- > This change is on top of the finit_module patch series. > --- > kernel/module.c | 5 ++++- > 1 files changed, 4 insertions(+), 1 deletions(-) > > diff --git a/kernel/module.c b/kernel/module.c > index 0ad03c4..05b8dde 100644 > --- a/kernel/module.c > +++ b/kernel/module.c > @@ -2441,8 +2441,11 @@ int copy_module_from_user(const void __user *umod, unsigned long len, > return -ENOMEM; > > err = copy_from_user(info->hdr, umod, info->len); > - if (err) > + if (err) { > + if (err > 0) ^^^^^^^^^^^ This condition is always true because copy_to/from_user() returns the number of bytes remaining to be copied. (It never returns a negative error code). > + err = -EFAULT; > goto free_hdr; > + } regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html