Re: [Xen-devel] memory corruption in HYPERVISOR_physdev_op()

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2012-09-14 at 14:24 +0300, Dan Carpenter wrote:
> Hi Jeremy,

Jeremy doesn't work on Xen much any more. Adding Konrad and the
xen-devel@ list.

> My static analyzer complains about potential memory corruption in
> HYPERVISOR_physdev_op()
> 
> arch/x86/include/asm/xen/hypercall.h
>    389  static inline int
>    390  HYPERVISOR_physdev_op(int cmd, void *arg)
>    391  {
>    392          int rc = _hypercall2(int, physdev_op, cmd, arg);
>    393          if (unlikely(rc == -ENOSYS)) {
>    394                  struct physdev_op op;
>    395                  op.cmd = cmd;
>    396                  memcpy(&op.u, arg, sizeof(op.u));
>    397                  rc = _hypercall1(int, physdev_op_compat, &op);
>    398                  memcpy(arg, &op.u, sizeof(op.u));
>                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Some of the arg buffers are not as large as sizeof(op.u) which is either
> 12 or 16 depending on the size of longs in struct physdev_apic.

Nasty!

> 
>    399          }
>    400          return rc;
>    401  }
> 
> One example of this is in xen_initdom_restore_msi_irqs().
> 
> arch/x86/pci/xen.c
>    337                  struct physdev_pci_device restore_ext;
>    338  
>    339                  restore_ext.seg = pci_domain_nr(dev->bus);
>    340                  restore_ext.bus = dev->bus->number;
>    341                  restore_ext.devfn = dev->devfn;
>    342                  ret = HYPERVISOR_physdev_op(PHYSDEVOP_restore_msi_ext,
>    343                                          &restore_ext);
>                                                 ^^^^^^^^^^^^
> There are only 4 bytes here.
> 
>    344                  if (ret == -ENOSYS)
>                             ^^^^^^^^^^^^^^
> If we hit this condition, we have corrupted some memory.

I can see the memory corruption but how does it relate to ret ==
-ENOSYS?

> 
>    345                          pci_seg_supported = false;
> 
> regards,
> dan carpenter
> _______________________________________________
> Virtualization mailing list
> Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
> https://lists.linuxfoundation.org/mailman/listinfo/virtualization
> 

-- 
Ian Campbell
Current Noise: Therapy? - Femtex

Riffle West Virginia is so small that the Boy Scout had to double as the
town drunk.


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