Messages in this thread | | | Date | Thu, 30 Jul 2020 14:38:52 +0200 | From | Joerg Roedel <> | Subject | Re: [PATCH 1/4] KVM: SVM: nested: Don't allocate VMCB structures on stack |
| |
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
| |