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@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel

Reply via email to