Skip to content

Commit

Permalink
vhost: fix to check the return value of copy_to/from_user() correctly
Browse files Browse the repository at this point in the history
copy_to/from_user() returns the number of bytes that could not be copied.

So we need to check if it is not zero, and in that case, we should return
the error number -EFAULT rather than directly return the return value from
copy_to/from_user().

Signed-off-by: Takuya Yoshikawa <[email protected]>
Signed-off-by: Michael S. Tsirkin <[email protected]>
  • Loading branch information
Takuya Yoshikawa authored and mstsirkin committed May 27, 2010
1 parent 0f3d9a1 commit 7ad9c9d
Showing 1 changed file with 28 additions and 23 deletions.
51 changes: 28 additions & 23 deletions drivers/vhost/vhost.c
Original file line number Diff line number Diff line change
Expand Up @@ -320,10 +320,8 @@ static long vhost_set_memory(struct vhost_dev *d, struct vhost_memory __user *m)
{
struct vhost_memory mem, *newmem, *oldmem;
unsigned long size = offsetof(struct vhost_memory, regions);
long r;
r = copy_from_user(&mem, m, size);
if (r)
return r;
if (copy_from_user(&mem, m, size))
return -EFAULT;
if (mem.padding)
return -EOPNOTSUPP;
if (mem.nregions > VHOST_MEMORY_MAX_NREGIONS)
Expand All @@ -333,11 +331,10 @@ static long vhost_set_memory(struct vhost_dev *d, struct vhost_memory __user *m)
return -ENOMEM;

memcpy(newmem, &mem, size);
r = copy_from_user(newmem->regions, m->regions,
mem.nregions * sizeof *m->regions);
if (r) {
if (copy_from_user(newmem->regions, m->regions,
mem.nregions * sizeof *m->regions)) {
kfree(newmem);
return r;
return -EFAULT;
}

if (!memory_access_ok(d, newmem, vhost_has_feature(d, VHOST_F_LOG_ALL)))
Expand Down Expand Up @@ -389,9 +386,10 @@ static long vhost_set_vring(struct vhost_dev *d, int ioctl, void __user *argp)
r = -EBUSY;
break;
}
r = copy_from_user(&s, argp, sizeof s);
if (r < 0)
if (copy_from_user(&s, argp, sizeof s)) {
r = -EFAULT;
break;
}
if (!s.num || s.num > 0xffff || (s.num & (s.num - 1))) {
r = -EINVAL;
break;
Expand All @@ -405,9 +403,10 @@ static long vhost_set_vring(struct vhost_dev *d, int ioctl, void __user *argp)
r = -EBUSY;
break;
}
r = copy_from_user(&s, argp, sizeof s);
if (r < 0)
if (copy_from_user(&s, argp, sizeof s)) {
r = -EFAULT;
break;
}
if (s.num > 0xffff) {
r = -EINVAL;
break;
Expand All @@ -419,12 +418,14 @@ static long vhost_set_vring(struct vhost_dev *d, int ioctl, void __user *argp)
case VHOST_GET_VRING_BASE:
s.index = idx;
s.num = vq->last_avail_idx;
r = copy_to_user(argp, &s, sizeof s);
if (copy_to_user(argp, &s, sizeof s))
r = -EFAULT;
break;
case VHOST_SET_VRING_ADDR:
r = copy_from_user(&a, argp, sizeof a);
if (r < 0)
if (copy_from_user(&a, argp, sizeof a)) {
r = -EFAULT;
break;
}
if (a.flags & ~(0x1 << VHOST_VRING_F_LOG)) {
r = -EOPNOTSUPP;
break;
Expand Down Expand Up @@ -477,9 +478,10 @@ static long vhost_set_vring(struct vhost_dev *d, int ioctl, void __user *argp)
vq->used = (void __user *)(unsigned long)a.used_user_addr;
break;
case VHOST_SET_VRING_KICK:
r = copy_from_user(&f, argp, sizeof f);
if (r < 0)
if (copy_from_user(&f, argp, sizeof f)) {
r = -EFAULT;
break;
}
eventfp = f.fd == -1 ? NULL : eventfd_fget(f.fd);
if (IS_ERR(eventfp)) {
r = PTR_ERR(eventfp);
Expand All @@ -492,9 +494,10 @@ static long vhost_set_vring(struct vhost_dev *d, int ioctl, void __user *argp)
filep = eventfp;
break;
case VHOST_SET_VRING_CALL:
r = copy_from_user(&f, argp, sizeof f);
if (r < 0)
if (copy_from_user(&f, argp, sizeof f)) {
r = -EFAULT;
break;
}
eventfp = f.fd == -1 ? NULL : eventfd_fget(f.fd);
if (IS_ERR(eventfp)) {
r = PTR_ERR(eventfp);
Expand All @@ -510,9 +513,10 @@ static long vhost_set_vring(struct vhost_dev *d, int ioctl, void __user *argp)
filep = eventfp;
break;
case VHOST_SET_VRING_ERR:
r = copy_from_user(&f, argp, sizeof f);
if (r < 0)
if (copy_from_user(&f, argp, sizeof f)) {
r = -EFAULT;
break;
}
eventfp = f.fd == -1 ? NULL : eventfd_fget(f.fd);
if (IS_ERR(eventfp)) {
r = PTR_ERR(eventfp);
Expand Down Expand Up @@ -575,9 +579,10 @@ long vhost_dev_ioctl(struct vhost_dev *d, unsigned int ioctl, unsigned long arg)
r = vhost_set_memory(d, argp);
break;
case VHOST_SET_LOG_BASE:
r = copy_from_user(&p, argp, sizeof p);
if (r < 0)
if (copy_from_user(&p, argp, sizeof p)) {
r = -EFAULT;
break;
}
if ((u64)(unsigned long)p != p) {
r = -EFAULT;
break;
Expand Down

0 comments on commit 7ad9c9d

Please sign in to comment.