Re: [PATCH V2] xen/hvm: fix hypervisor crash with hvm_save_one()

[Tim Deegan <tim@xen.org>]

At 18:21 +0300 on 02 May (1493749307), Razvan Cojocaru wrote:
> hvm_save_cpu_ctxt() returns success without writing any data into
> hvm_domain_context_t when all VCPUs are offline. This can then crash
> the hypervisor (with FATAL PAGE FAULT) in hvm_save_one() via the
> "off < (ctxt.cur - sizeof(*desc))" for() test, where ctxt.cur remains 0,
> causing an underflow which leads the hypervisor to go off the end of the
> ctxt buffer.
[...]
> Reported-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
> Tested-by: Razvan Cojocaru <rcojocaru@bitdefender.com>

I actually preferred the first patch :P but this is fine.

Acked-by: Tim Deegan <tim@xen.org>

> diff --git a/xen/common/hvm/save.c b/xen/common/hvm/save.c
> index 78706f5..3bdd124 100644
> --- a/xen/common/hvm/save.c
> +++ b/xen/common/hvm/save.c
> @@ -113,6 +113,9 @@ int hvm_save_one(struct domain *d, uint16_t typecode, uint16_t instance,
>          const struct hvm_save_descriptor *desc;
>  
>          rv = -ENOENT;
> +        if ( ctxt.cur < sizeof(*desc) )
> +            goto out;
> +
>          for ( off = 0; off < (ctxt.cur - sizeof(*desc)); off += desc->length )
>          {
>              desc = (void *)(ctxt.data + off);
> @@ -132,6 +135,7 @@ int hvm_save_one(struct domain *d, uint16_t typecode, uint16_t instance,
>          }
>      }
>  
> + out:
>      xfree(ctxt.data);
>      return rv;
>  }
> -- 
> 1.9.1
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel