First of all, I'm happy that you're reviewing these patches. A lot of
learner, often untested, patches go through kernel-janitors and the
entire purpose of the list is to check each other's work.
Here is what I meant by an information leak. The original code did:
spid = kmalloc(SCIOC_SPIDSIZE, GFP_KERNEL);
...
strcpy(spid, rcvmsg->msg_data.byte_array);
...
if (copy_to_user(data->dataptr, spid, SCIOC_SPIDSIZE)) {
When you allocate memory with kmalloc() it doesn't clear that memory. It
could have anything in there including passwords etc. If there is a
short string in "rcvmsg->msg_data.byte_array" like "123" then the
strcpy() puts that in the first 4 bytes of "spid", but the rest of the
buffer is still full of passwords.
The copy_to_user() sends it to the user, and normally the user only reads
as far as the terminator, but if they read past that then they would see
all the passwords etc that were saved there.
This function is in the ioctl() probably only root has open() permission
on the device so it's not a big deal because root can already find your
password. But I didn't dig that far. It's just simpler to zero out the
memory instead of worrying about if it really is exploitable or not.
Also it's quite possible that the strcpy can't overflow. But it's weird
for the code to leave space for the terminator and then not use it. In
the end, I decided to do the cautious thing and be done with it.
regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
