Re: [PATCH 2.4] i2c-dev user/kernel bug and mem leak

From: Jean Delvare (khali@linux-fr.org)
Date: Wed Aug 06 2003 - 03:07:02 EST


Hi Marcelo,

Greg> Patch looks good, want to send it to Marcelo, or do you want me
Greg> to?

Here I do, speaking in the name of the I2C & LM Sensors development team
:)

A patch to i2c-dev follows, built against 2.4.22-pre10, which brings
the following changes:

* Fix a user/kernel bug discovered by Robert T. Johnson.
  Patch by Sergey Vlasov.
* Fix a memory leak discovered by Robert T. Johnson.
  Patch by Sergey Vlasov and me.
* Two code optimizations/cleanups.
  Patches by Sergey Vlaslov and me, respectively.
* Bonus: two lines with changed whitespace, but you don't care.
  Patches by me and Robert T. Johnson, respectively.

These changes should also be made to i2c-CVS and Linux 2.6's i2c
subsystem. I will commit the changes to i2c-CVS while Greg KH will
submit a modified patch to Linus.

The final patch was tested by me (compiles and runs with no apparent
drawback) and approved by Sergey Vlaslov and Greg KH.
Please apply.

Thanks,
Jean

--- 2.4/drivers/i2c/i2c-dev.c Tue Jul 15 12:23:49 2003
+++ 2.4/drivers/i2c/i2c-dev.c Wed Aug 6 09:36:54 2003
@@ -219,6 +219,7 @@
         struct i2c_smbus_ioctl_data data_arg;
         union i2c_smbus_data temp;
         struct i2c_msg *rdwr_pa;
+ u8 **data_ptrs;
         int i,datasize,res;
         unsigned long funcs;
 
@@ -248,7 +249,7 @@
                 return (copy_to_user((unsigned long *)arg,&funcs,
                                      sizeof(unsigned long)))?-EFAULT:0;
 
- case I2C_RDWR:
+ case I2C_RDWR:
                 if (copy_from_user(&rdwr_arg,
                                    (struct i2c_rdwr_ioctl_data *)arg,
                                    sizeof(rdwr_arg)))
@@ -265,21 +266,28 @@
 
                 if (rdwr_pa == NULL) return -ENOMEM;
 
+ if (copy_from_user(rdwr_pa, rdwr_arg.msgs,
+ rdwr_arg.nmsgs * sizeof(struct i2c_msg))) {
+ kfree(rdwr_pa);
+ return -EFAULT;
+ }
+
+ data_ptrs = (u8 **) kmalloc(rdwr_arg.nmsgs * sizeof(u8 *),
+ GFP_KERNEL);
+ if (data_ptrs == NULL) {
+ kfree(rdwr_pa);
+ return -ENOMEM;
+ }
+
                 res = 0;
                 for( i=0; i<rdwr_arg.nmsgs; i++ )
                 {
- if(copy_from_user(&(rdwr_pa[i]),
- &(rdwr_arg.msgs[i]),
- sizeof(rdwr_pa[i])))
- {
- res = -EFAULT;
- break;
- }
                         /* Limit the size of the message to a sane amount */
                         if (rdwr_pa[i].len > 8192) {
                                 res = -EINVAL;
                                 break;
                         }
+ data_ptrs[i] = rdwr_pa[i].buf;
                         rdwr_pa[i].buf = kmalloc(rdwr_pa[i].len, GFP_KERNEL);
                         if(rdwr_pa[i].buf == NULL)
                         {
@@ -287,10 +295,11 @@
                                 break;
                         }
                         if(copy_from_user(rdwr_pa[i].buf,
- rdwr_arg.msgs[i].buf,
+ data_ptrs[i],
                                 rdwr_pa[i].len))
                         {
- res = -EFAULT;
+ ++i; /* Needs to be kfreed too */
+ res = -EFAULT;
                                 break;
                         }
                 }
@@ -298,21 +307,20 @@
                         int j;
                         for (j = 0; j < i; ++j)
                                 kfree(rdwr_pa[j].buf);
+ kfree(data_ptrs);
                         kfree(rdwr_pa);
                         return res;
                 }
- if (!res)
- {
- res = i2c_transfer(client->adapter,
- rdwr_pa,
- rdwr_arg.nmsgs);
- }
+
+ res = i2c_transfer(client->adapter,
+ rdwr_pa,
+ rdwr_arg.nmsgs);
                 while(i-- > 0)
                 {
                         if( res>=0 && (rdwr_pa[i].flags & I2C_M_RD))
                         {
                                 if(copy_to_user(
- rdwr_arg.msgs[i].buf,
+ data_ptrs[i],
                                         rdwr_pa[i].buf,
                                         rdwr_pa[i].len))
                                 {
@@ -321,6 +329,7 @@
                         }
                         kfree(rdwr_pa[i].buf);
                 }
+ kfree(data_ptrs);
                 kfree(rdwr_pa);
                 return res;
 

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Thu Aug 07 2003 - 22:00:32 EST