Messages in this thread | | | From | Frodo Looijaard <> | Subject | Re: Suspicious code in i2c-dev.c | Date | Sun, 23 Jan 2000 22:44:07 +0100 (CET) |
| |
Russell King wrote:
> Looking through i2c-dev.c, I noticed the following code, which appears > to be wrong: > > /* copy user space data to kernel space. */ > tmp = kmalloc(count,GFP_KERNEL); > if (tmp==NULL) > return -ENOMEM; > > #ifdef DEBUG > printk("i2c-dev,o: i2c-%d reading %d bytes.\n",MINOR(inode->i_rdev),count); > #endif > > ret = i2c_master_recv(client,tmp,count); > copy_to_user(buf,tmp,count); > kfree(tmp); > return ret; > > for the following reasons: > > 1. If i2c_master_recv does not write to `tmp' but returns an error, > copy_to_user will copy uninitialised kernel memory to the user, > possibly from another user process - security concern.
Granted. Easily solved with a test whether ret equals zero at this place. I have applied this to our master archive; we'll include it in the next diffs we send (probably tomorrow).
> 2. copy_to_user may fault, and if this is the case, the user will > never know (the user will not see the EFAULT error code returned).
> There are more instances of point 2, including some with copy_from_user.
Hmm. Can you elaborate on that? What exactly will happen when a copy_{to,from}_user faults? And what does a fault in this instance mean (cache miss?).
Thanks, Frodo
-- Frodo Looijaard <frodol@dds.nl> PGP key and more: http://huizen.dds.nl/~frodol Defenestration n. (formal or joc.): The act of removing Windows from your computer in disgust, usually followed by the installation of Linux or some other Unix-like operating system.
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/
| |