[PATCH][2.6.11-mm3] Warning fix and be extra careful about array in kernel/module.c

2005-03-13 Thread Jesper Juhl

Fix warning in kernel/module.c::who_is_doing_it()
kernel/module.c:1405: warning: ignoring return value of `copy_from_user', 
declared with attribute warn_unused_result
by subtracting copy_from_user return value from 'len' - if we copy less 
data than we intend there's no point in looping over more than we actually 
copy.
I've also changed the type of 'len' and 'i' from unsigned int to 
unsigned long since that's the type of mm_struct.arg_start/arg_end and 
it's also the type of the last argument to copy_from_user. Sure, it'll be 
promoted and truncated between int/long just fine, but it seems cleaner to 
me that it's just the same type all the way.
I also added an explicit check of the value of 'len' after the calcolation 
of arg_start - arg_end since they are both long values subtracting one 
from the other could theoretically ield a value larger than 512 and such a 
value would cause us to overflow our statically allocated array. I don't 
know if that can actually ever occour, I don't know the code that well, 
but I thought it would be better to be safe than sorry.

The code compiles and I've been running the kernel with this change for a 
few hrs, but that's all the testing I've done.

If this seems OK to you, please consider merging it. If there's something 
wrong with it I'm always looking forward to being enlightened.


Signed-off-by: Jesper Juhl <[EMAIL PROTECTED]>

diff -up linux-2.6.11-mm3-orig/kernel/module.c linux-2.6.11-mm3/kernel/module.c
--- linux-2.6.11-mm3-orig/kernel/module.c   2005-03-13 00:52:51.0 
+0100
+++ linux-2.6.11-mm3/kernel/module.c2005-03-13 01:51:11.0 +0100
@@ -1400,9 +1400,12 @@ static void who_is_doing_it(void)
 {
/* Print out all the args. */
char args[512];
-   unsigned int i, len = current->mm->arg_end - current->mm->arg_start;
+   unsigned long i, len = current->mm->arg_end - current->mm->arg_start;
 
-   copy_from_user(args, (void *)current->mm->arg_start, len);
+   if (len > 512)
+   len = 512;
+
+   len -= copy_from_user(args, (void *)current->mm->arg_start, len);
 
for (i = 0; i < len; i++) {
if (args[i] == '\0')



-
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][2.6.11-mm3] Warning fix and be extra careful about array in kernel/module.c

2005-03-13 Thread Jesper Juhl

Fix warning in kernel/module.c::who_is_doing_it()
kernel/module.c:1405: warning: ignoring return value of `copy_from_user', 
declared with attribute warn_unused_result
by subtracting copy_from_user return value from 'len' - if we copy less 
data than we intend there's no point in looping over more than we actually 
copy.
I've also changed the type of 'len' and 'i' from unsigned int to 
unsigned long since that's the type of mm_struct.arg_start/arg_end and 
it's also the type of the last argument to copy_from_user. Sure, it'll be 
promoted and truncated between int/long just fine, but it seems cleaner to 
me that it's just the same type all the way.
I also added an explicit check of the value of 'len' after the calcolation 
of arg_start - arg_end since they are both long values subtracting one 
from the other could theoretically ield a value larger than 512 and such a 
value would cause us to overflow our statically allocated array. I don't 
know if that can actually ever occour, I don't know the code that well, 
but I thought it would be better to be safe than sorry.

The code compiles and I've been running the kernel with this change for a 
few hrs, but that's all the testing I've done.

If this seems OK to you, please consider merging it. If there's something 
wrong with it I'm always looking forward to being enlightened.


Signed-off-by: Jesper Juhl [EMAIL PROTECTED]

diff -up linux-2.6.11-mm3-orig/kernel/module.c linux-2.6.11-mm3/kernel/module.c
--- linux-2.6.11-mm3-orig/kernel/module.c   2005-03-13 00:52:51.0 
+0100
+++ linux-2.6.11-mm3/kernel/module.c2005-03-13 01:51:11.0 +0100
@@ -1400,9 +1400,12 @@ static void who_is_doing_it(void)
 {
/* Print out all the args. */
char args[512];
-   unsigned int i, len = current-mm-arg_end - current-mm-arg_start;
+   unsigned long i, len = current-mm-arg_end - current-mm-arg_start;
 
-   copy_from_user(args, (void *)current-mm-arg_start, len);
+   if (len  512)
+   len = 512;
+
+   len -= copy_from_user(args, (void *)current-mm-arg_start, len);
 
for (i = 0; i  len; i++) {
if (args[i] == '\0')



-
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/