[PATCH 4/9] staging/lustre: clean up and remove libcfs/linux/linux-fs.c
Peng Tao
bergwolf at gmail.com
Tue Jun 4 10:23:40 UTC 2013
On Tue, Jun 4, 2013 at 4:32 PM, Dan Carpenter <dan.carpenter at oracle.com> wrote:
> 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?
>
Thanks for reviewing. I am not familiar with access_ok() but I think
you are right and I'll test to see if it really breaks. In the
meantime, I took a look at other kernel callers of vfs_write() and
btrfs is also calling vfs_write() to write kernel stack memory
(fs/btrfs/send.c: send_header->write_buf->vfs_write). So I CC'ed btrfs
list in case someone knows better than I do.
> 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.
>
Thanks. I will fix it.
> 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.
>
For partial writes, both payload and count are advanced properly. So
it won't loop forever.
Thanks,
Tao
More information about the devel
mailing list