On Wed, May 08, 2013 at 10:53:28AM +0100, Marc Zyngier wrote:
> On 07/05/13 13:55, Andre Przywara wrote:
> > Some ARM KVM VCPU ioctls require the vCPU to be properly initialized
> > with the KVM_ARM_VCPU_INIT ioctl before being used with further
> > requests. KVM_RUN checks whether this initialization has been
> > done, but other ioctls do not.
> > Namely KVM_GET_REG_LIST will dereference an array with index -1
> > without initialization and thus leads to a kernel oops.
> > Fix this by adding checks before executing the ioctl handlers.
> >
> > Signed-off-by: Andre Przywara <andre.przywara@xxxxxxxxxx>
> > ---
> > arch/arm/kvm/arm.c | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> > index c1fe498..0c571ff 100644
> > --- a/arch/arm/kvm/arm.c
> > +++ b/arch/arm/kvm/arm.c
> > @@ -893,6 +893,11 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
> > case KVM_SET_ONE_REG:
> > case KVM_GET_ONE_REG: {
> > struct kvm_one_reg reg;
> > +
> > + /* Make sure they initialize the vcpu with KVM_ARM_VCPU_INIT */
> > + if (unlikely(vcpu->arch.target < 0))
> > + return -ENOEXEC;
> > +
> > if (copy_from_user(®, argp, sizeof(reg)))
> > return -EFAULT;
> > if (ioctl == KVM_SET_ONE_REG)
> > @@ -905,6 +910,10 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
> > struct kvm_reg_list reg_list;
> > unsigned n;
> >
> > + /* Make sure they initialize the vcpu with KVM_ARM_VCPU_INIT */
> > + if (unlikely(vcpu->arch.target < 0))
> > + return -ENOEXEC;
> > +
> > if (copy_from_user(®_list, user_list, sizeof(reg_list)))
> > return -EFAULT;
> > n = reg_list.n;
> >
>
> So we also have the same snippet in kvm_arch_vcpu_ioctl_run().
>
> Can you create a static function (kvm_vcpu_initialized?) containing that
> code and make all three sites use it?
>
Andre, will you re-spin your patch as per Marc's comments?
Thanks,
-Christoffer
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
