On Fri, 2003-08-15 at 14:13, Greg KH wrote: > Hm, I had already applied your patch, so this one doesn't apply. Care > to re-do it against 2.6.0-test4 whenever that comes out? Here's the patch against 2.6.0-test4. Just to remind everyone, this patch doesn't fix any bugs (they're already fixed in 2.6.0-test3), it just makes the code pass our static analysis tool, cqual, without generating a warning. Since finding and fixing these bugs is so tricky, it seems worthwhile to have code which can be automatically verified to be bug-free (at least w.r.t. user/kernel pointers). That's what this patch is about. Let me know if you have any questions or comments. Thanks for everyone's help. Best, Rob P.S. cqual is on its way -- we're working on documentation and integrating it into the kernel build process. --- drivers/i2c/i2c-dev.c.orig Wed Aug 27 18:04:31 2003 +++ drivers/i2c/i2c-dev.c Wed Aug 27 17:51:23 2003 @@ -181,7 +181,7 @@ struct i2c_smbus_ioctl_data data_arg; union i2c_smbus_data temp; struct i2c_msg *rdwr_pa; - u8 **data_ptrs; + struct i2c_msg *tmp_pa; int i,datasize,res; unsigned long funcs; @@ -226,40 +226,44 @@ if (rdwr_arg.nmsgs > 42) return -EINVAL; - rdwr_pa = (struct i2c_msg *) + tmp_pa = (struct i2c_msg *) kmalloc(rdwr_arg.nmsgs * sizeof(struct i2c_msg), GFP_KERNEL); - if (rdwr_pa == NULL) return -ENOMEM; + if (tmp_pa == NULL) return -ENOMEM; - if (copy_from_user(rdwr_pa, rdwr_arg.msgs, + if (copy_from_user(tmp_pa, rdwr_arg.msgs, rdwr_arg.nmsgs * sizeof(struct i2c_msg))) { - kfree(rdwr_pa); + kfree(tmp_pa); return -EFAULT; } - data_ptrs = (u8 **) kmalloc(rdwr_arg.nmsgs * sizeof(u8 *), - GFP_KERNEL); - if (data_ptrs == NULL) { - kfree(rdwr_pa); + rdwr_pa = (struct i2c_msg *) + kmalloc(rdwr_arg.nmsgs * sizeof(struct i2c_msg), + GFP_KERNEL); + + if (rdwr_pa == NULL) { + kfree(tmp_pa); return -ENOMEM; } res = 0; for( i=0; i<rdwr_arg.nmsgs; i++ ) { + rdwr_pa[i].addr = tmp_pa[i].addr; + rdwr_pa[i].flags = tmp_pa[i].flags; + rdwr_pa[i].len = tmp_pa[i].len; /* 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) { res = -ENOMEM; break; } if(copy_from_user(rdwr_pa[i].buf, - data_ptrs[i], + tmp_pa[i].buf, rdwr_pa[i].len)) { ++i; /* Needs to be kfreed too */ res = -EFAULT; @@ -270,8 +274,8 @@ int j; for (j = 0; j < i; ++j) kfree(rdwr_pa[j].buf); - kfree(data_ptrs); kfree(rdwr_pa); + kfree(tmp_pa); return res; } @@ -281,7 +285,7 @@ while(i-- > 0) { if( res>=0 && (rdwr_pa[i].flags & I2C_M_RD)) { if(copy_to_user( - data_ptrs[i], + tmp_pa[i].buf, rdwr_pa[i].buf, rdwr_pa[i].len)) { res = -EFAULT; @@ -289,8 +293,8 @@ } kfree(rdwr_pa[i].buf); } - kfree(data_ptrs); kfree(rdwr_pa); + kfree(tmp_pa); return res; case I2C_SMBUS: