On Mon, Oct 17, 2011 at 11:27:40AM +0200, Jan Kiszka wrote:
> Only accesses to the MSI-X table must trigger a call to
> msix_handle_mask_update or a notifier invocation.
>
> Signed-off-by: Jan Kiszka <[email protected]>
Why would msix_mmio_write be called on an access
outside the table?
> ---
> hw/msix.c | 16 ++++++++++------
> 1 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/hw/msix.c b/hw/msix.c
> index 2c4de21..33cb716 100644
> --- a/hw/msix.c
> +++ b/hw/msix.c
> @@ -264,18 +264,22 @@ static void msix_mmio_write(void *opaque,
> target_phys_addr_t addr,
> {
> PCIDevice *dev = opaque;
> unsigned int offset = addr & (MSIX_PAGE_SIZE - 1) & ~0x3;
> - int vector = offset / PCI_MSIX_ENTRY_SIZE;
> + unsigned int vector = offset / PCI_MSIX_ENTRY_SIZE;
Why the int/unsigned change? this has no chance to overflow, and using
unsigned causes signed/unsigned comparison below,
and unsigned/signed conversion on calls such as msix_is_masked.
> int was_masked = msix_is_masked(dev, vector);
> pci_set_long(dev->msix_table_page + offset, val);
> if (kvm_enabled() && kvm_irqchip_in_kernel()) {
> kvm_msix_update(dev, vector, was_masked, msix_is_masked(dev,
> vector));
> }
I would say if we need to check the address, check it first thing
and return if the address is out of a sensible range.
For example, are you worried about kvm_msix_update calls with
a sensible mask?
> - if (was_masked != msix_is_masked(dev, vector) &&
> dev->msix_mask_notifier) {
> - int r = dev->msix_mask_notifier(dev, vector,
> - msix_is_masked(dev, vector));
> - assert(r >= 0);
> +
> + if (vector < dev->msix_entries_nr) {
> + if (was_masked != msix_is_masked(dev, vector) &&
> + dev->msix_mask_notifier) {
> + int r = dev->msix_mask_notifier(dev, vector,
> + msix_is_masked(dev, vector));
> + assert(r >= 0);
> + }
> + msix_handle_mask_update(dev, vector);
> }
> - msix_handle_mask_update(dev, vector);
> }
>
> static const MemoryRegionOps msix_mmio_ops = {
> --
> 1.7.3.4
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
