[pve-devel] [PATCH pve-qemu-kvm] fix #991: Can't install Win7/8

[Thomas Lamprecht]

comments inline

On 05/11/2016 02:30 PM, Wolfgang Link wrote:
> ---
>  Makefile | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index 67b30dd..07cd162 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -24,7 +24,8 @@ download:
>  	#git clone git://git.qemu-project.org/qemu.git -b stable-2.4 ${KVMDIR} 
>  	git clone git://git.qemu-project.org/qemu.git ${KVMDIR}
>  	# see https://bugs.launchpad.net/qemu/+bug/1488363?comments=all
> -	cd ${KVMDIR}; git checkout v2.5.1.1; git revert --no-edit b8eb5512fd8a115f164edbbe897cdf8884920ccb
> +	# see for 44b https://bugzilla.proxmox.com/show_bug.cgi?id=991
> +	cd ${KVMDIR}; git checkout v2.5.1.1; git revert --no-edit b8eb5512fd8a115f164edbbe897cdf8884920ccb; git revert --no-edit 44b86aa32e4147c727fadd9a0f0bc503a5dedb72;

The second commit reverts a CVE fix? Its a fix for a DoS of the guest so
it may be seen as less critical but still, is upstream informed on this,
I mean if this one breaks win7/8 also in qemu 2.6 this could be a
blocker for them...

Respective commit is:

> commit 44b86aa32e4147c727fadd9a0f0bc503a5dedb72
> Author: Gerd Hoffmann <kraxel at redhat.com>
> Date:   Tue Apr 26 14:48:06 2016 +0200
>
>     vga: make sure vga register setup for vbe stays intact
> (CVE-2016-3712).
>    
>     Call vbe_update_vgaregs() when the guest touches GFX, SEQ or CRT
>     registers, to make sure the vga registers will always have the
>     values needed by vbe mode.  This makes sure the sanity checks
>     applied by vbe_fixup_regs() are effective.
>    
>     Without this guests can muck with shift_control, can turn on planar
>     vga modes or text mode emulation while VBE is active, making qemu
>     take code paths meant for CGA compatibility, but with the very
>     large display widths and heigts settable using VBE registers.
>    
>     Which is good for one or another buffer overflow.  Not that
>     critical as they typically read overflows happening somewhere
>     in the display code.  So guests can DoS by crashing qemu with a
>     segfault, but it is probably not possible to break out of the VM.
>    
>     Fixes: CVE-2016-3712
>     Reported-by: Zuozhi Fzz <zuozhi.fzz at alibaba-inc.com>
>     Reported-by: P J P <ppandit at redhat.com>
>     Signed-off-by: Gerd Hoffmann <kraxel at redhat.com>
>     Signed-off-by: Michael Roth <mdroth at linux.vnet.ibm.com>
>
> diff --git a/hw/display/vga.c b/hw/display/vga.c
> index 10ac7df..679070e 100644
> --- a/hw/display/vga.c
> +++ b/hw/display/vga.c
> @@ -140,6 +140,8 @@ static uint32_t expand4[256];
>  static uint16_t expand2[256];
>  static uint8_t expand4to8[16];
>  
> +static void vbe_update_vgaregs(VGACommonState *s);
> +
>  static inline bool vbe_enabled(VGACommonState *s)
>  {
>      return s->vbe_regs[VBE_DISPI_INDEX_ENABLE] & VBE_DISPI_ENABLED;
> @@ -482,6 +484,7 @@ void vga_ioport_write(void *opaque, uint32_t addr,
> uint32_t val)
>          printf("vga: write SR%x = 0x%02x\n", s->sr_index, val);
>  #endif
>          s->sr[s->sr_index] = val & sr_mask[s->sr_index];
> +        vbe_update_vgaregs(s);
>          if (s->sr_index == VGA_SEQ_CLOCK_MODE) {
>              s->update_retrace_info(s);
>          }
> @@ -513,6 +516,7 @@ void vga_ioport_write(void *opaque, uint32_t addr,
> uint32_t val)
>          printf("vga: write GR%x = 0x%02x\n", s->gr_index, val);
>  #endif
>          s->gr[s->gr_index] = val & gr_mask[s->gr_index];
> +        vbe_update_vgaregs(s);
>          vga_update_memory_access(s);
>          break;
>      case VGA_CRT_IM:
> @@ -531,10 +535,12 @@ void vga_ioport_write(void *opaque, uint32_t
> addr, uint32_t val)
>              if (s->cr_index == VGA_CRTC_OVERFLOW) {
>                  s->cr[VGA_CRTC_OVERFLOW] = (s->cr[VGA_CRTC_OVERFLOW]
> & ~0x10) |
>                      (val & 0x10);
> +                vbe_update_vgaregs(s);
>              }
>              return;
>          }
>          s->cr[s->cr_index] = val;
> +        vbe_update_vgaregs(s);
>  
>          switch(s->cr_index) {
>          case VGA_CRTC_H_TOTAL:
>

I'll do some checks on QEMU 2.6-rc5 which includes this also.
For the future, could you please explain in the CI message which commit
you revert and why, not that we open a security hole because of a
"workaround" by accident.

Thanks,
Thomas

>  	tar czf ${KVMSRC} --exclude CVS --exclude .git --exclude .svn ${KVMDIR}
>  
>  ${DEBS} kvm: ${KVMSRC}