On Mon, Jun 03, 2013 at 09:58:17PM +0800, Peng Tao wrote:
> int libcfs_kkuc_msg_put(struct file *filp, void *payload)
> {
> struct kuc_hdr *kuch = (struct kuc_hdr *)payload;
> + ssize_t count = kuch->kuc_msglen;
> + loff_t offset = 0;
> + mm_segment_t fs;
> int rc = -ENOSYS;
>
> if (filp == NULL || IS_ERR(filp))
> @@ -165,11 +168,18 @@ int libcfs_kkuc_msg_put(struct file *filp, void *payload)
> return -ENOSYS;
> }
>
> - {
> - loff_t offset = 0;
> - rc = filp_user_write(filp, payload, kuch->kuc_msglen,
> - &offset);
> + fs = get_fs();
> + set_fs(KERNEL_DS);
> + while ((ssize_t)count > 0) {
> + rc = vfs_write(filp, (const void __user *)payload,
> + count, &offset);
> + if (rc < 0)
> + break;
> + count -= rc;
> + payload += rc;
> + rc = 0;
> }
> + set_fs(fs);
>
> if (rc < 0)
> CWARN("message send failed (%d)\n", rc);
This was all there in the original code, it has just been shifted.
Still, I had some questions about it. "payload" is a pointer to
kernel stack memory, wouldn't the access_ok() check in vfs_write()
fail every time on x86?
Some of the casting is not needed. No need to cast "count" because
it is already ssize_t. I haven't tested but I think Sparse will
complain about the __user cast until __force is added. No need for
the cast to const.
I worry about partial writes and that this could be a forever loop
but I don't know enough about anything to say if that's possible.
Probably not.
regards,
dan carpenter
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
