Re: [PATCH]: Fix memory corruption in-kernel IOAPIC emulation

[Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org>]

Chris Lalancette wrote:
> Another version of the patch, done by changing the on-the-wire protocol
> as Avi suggested.  I've tested this with:
>
> old -> old - Migration works, but runs into the bug I'm trying to fix
> old -> new - Migration works, but runs into the bug I'm trying to fix
> new -> old - Migration fails gracefully with ioapic version mismatch
> new -> new - Migration works, and doesn't run into this particular bug
>   

>  
> -static void kvm_kernel_ioapic_load_from_user(IOAPICState *s)
> +static int kvm_kernel_ioapic_load_from_user(IOAPICState *s, int version_id)
>  {
>  #if defined(KVM_CAP_IRQCHIP) && defined(TARGET_I386)
>      struct kvm_irqchip chip;
>      struct kvm_ioapic_state *kioapic;
>      int i;
>  
> +    if (version_id > 2) {
> +        return -EINVAL;
> +    }
> +
>      chip.chip_id = KVM_IRQCHIP_IOAPIC;
>      kioapic = &chip.chip.ioapic;
>  
> +    if (version_id == 2) {
> +        kioapic->base_address = s->base_address;
> +        kioapic->irr = s->irr;
> +    }
> +
>   

Preferably this function would always set all variables; the caller 
should make sure all the values are sane (by initializing them to sane 
values on ioapic creation, and not modifying them when a version 1 
migration happens).  It's best to limit protocol change handling to the 
actual qemu save/restore routines.

>  static void ioapic_save(QEMUFile *f, void *opaque)
> @@ -1173,8 +1188,9 @@ static int ioapic_load(QEMUFile *f, void *opaque, int version_id)
>  {
>      IOAPICState *s = opaque;
>      int i;
> +    int ret;
>  
> -    if (version_id != 1)
> +    if (version_id > 2)
>          return -EINVAL;
>   

This allows version_id == 0 to sneak in, where it couldn't before.  It's 
not a problem in practice, but let's get it right.



-- 
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/