On Thu, Aug 5, 2010 at 9:55 PM, Dan Carpenter <error27@xxxxxxxxx> wrote:
> 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.
>
Thanks, Dan. I got it.
--
Regards,
Changli Gao(xiaosuo@xxxxxxxxx)
--
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
