On Mon, Sep 26, 2022 at 11:26:45AM +0100, Fuad Tabba wrote:
...
> > +
> > +- KVM_MEM_PRIVATE can be set to indicate a new slot has private memory backed by
> > + a file descirptor(fd) and the content of the private memory is invisible to
>
> s/descirptor/descriptor
Thanks.
...
> static long kvm_vm_ioctl(struct file *filp,
> > unsigned int ioctl, unsigned long arg)
> > {
> > @@ -4645,14 +4672,20 @@ static long kvm_vm_ioctl(struct file *filp,
> > break;
> > }
> > case KVM_SET_USER_MEMORY_REGION: {
> > - struct kvm_userspace_memory_region kvm_userspace_mem;
> > + struct kvm_user_mem_region mem;
> > + unsigned long size = sizeof(struct kvm_userspace_memory_region);
>
> nit: should this be sizeof(struct mem)? That's more similar to the
> existing code and makes it dependent on the size of mem regardless of
> possible changes to its type in the future.
Unluckily no, the size we need copy_from_user() depends on the flags,
e.g. without KVM_MEM_PRIVATE, we can't safely copy that big size since
the 'extended' part may not even exist.
>
> > +
> > + kvm_sanity_check_user_mem_region_alias();
> >
> > r = -EFAULT;
> > - if (copy_from_user(&kvm_userspace_mem, argp,
> > - sizeof(kvm_userspace_mem)))
> > + if (copy_from_user(&mem, argp, size);
>
> It gets fixed in a future patch, but the ; should be a ).
Good catch, thanks!
Chao
>
> Cheers,
> /fuad
