Hi Sean, thanks for your review! On Wed, Jul 29, 2020 at 08:14:55AM -0700, Sean Christopherson wrote: > On Wed, Jul 29, 2020 at 03:22:31PM +0200, Joerg Roedel wrote: > Speaking of too large, would it be overly paranoid to add: > > BUILD_BUG_ON(sizeof(struct vmcb_control_area) + sizeof(struct vmcb_save_area) < > KVM_STATE_NESTED_SVM_VMCB_SIZE) > > More so for documentation than for any real concern that the SVM architecture > will do something silly, e.g. to make it obvious that patch 2 in this series > won't break backwards compatibility. The check should actually be '>', but then it makes sense. The control- and save-area together are still way smaller than 4k. I will add the check for '>' to this patch. > > + ret = -EFAULT; > > + if (copy_from_user(ctl, &user_vmcb->control, sizeof(ctl))) > > The sizeof() calc is wrong, this is now calculating the size of the pointer, > not the size of the struct. It'd need to be sizeof(*ctl). > > > + goto out_free; > > + if (copy_from_user(save, &user_vmcb->save, sizeof(save))) > > Same bug here. Thanks, fixed that. Regards, Joerg
