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...@intel.com> > Signed-off-by: Kees Cook <keesc...@chromium.org> > --- > 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 linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/