Dan Carpenter <[email protected]> writes: > 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 <[email protected]> >> Signed-off-by: Kees Cook <[email protected]> >> --- >> 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).
Yes, I made the obvious fix (eliminating the >0 check). This "copy_from_user is stupid" was a debate a lost long ago, but it still annoys me. Applied, 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/
