* [PATCH] hw/pci/pci.c: Assert that pci_irq_handler() inputs are valid
@ 2021-03-23 16:46 Peter Maydell
2021-03-23 20:47 ` Philippe Mathieu-Daudé
2021-03-24 16:21 ` Isaku Yamahata
0 siblings, 2 replies; 3+ messages in thread
From: Peter Maydell @ 2021-03-23 16:46 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael S. Tsirkin
pci_irq_handler documents that it must be called with 0 <= irq_num <=
3 and level either 0 or 1. Add assertions that the caller has passed
us in valid arguments.
In particular, if a device model fails to set the PCI_INTERRUPT_PIN
field in its config space correctly to indicate that it has an
interrupt, and then tries to raise an interrupt (either by calling
pci_set_irq(), or by getting a qemu_irq from pci_allocate_irq() and
then calling qemu_set_irq() on that) we will now diagnose this device
model bug with an assertion rather than engaging in the undefined
behaviour of shifting by a negative number.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/pci/pci.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 0eadcdbc9ec..38aefe22ab3 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1449,6 +1449,9 @@ static void pci_irq_handler(void *opaque, int irq_num, int level)
PCIDevice *pci_dev = opaque;
int change;
+ assert(irq_num >= 0 && irq_num < PCI_NUM_PINS);
+ assert(level == 0 || level == 1);
+
change = level - pci_irq_state(pci_dev, irq_num);
if (!change)
return;
--
2.20.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] hw/pci/pci.c: Assert that pci_irq_handler() inputs are valid
2021-03-23 16:46 [PATCH] hw/pci/pci.c: Assert that pci_irq_handler() inputs are valid Peter Maydell
@ 2021-03-23 20:47 ` Philippe Mathieu-Daudé
2021-03-24 16:21 ` Isaku Yamahata
1 sibling, 0 replies; 3+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-23 20:47 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: Michael S. Tsirkin
On 3/23/21 5:46 PM, Peter Maydell wrote:
> pci_irq_handler documents that it must be called with 0 <= irq_num <=
> 3 and level either 0 or 1. Add assertions that the caller has passed
> us in valid arguments.
>
> In particular, if a device model fails to set the PCI_INTERRUPT_PIN
> field in its config space correctly to indicate that it has an
> interrupt, and then tries to raise an interrupt (either by calling
> pci_set_irq(), or by getting a qemu_irq from pci_allocate_irq() and
> then calling qemu_set_irq() on that) we will now diagnose this device
> model bug with an assertion rather than engaging in the undefined
> behaviour of shifting by a negative number.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> hw/pci/pci.c | 3 +++
> 1 file changed, 3 insertions(+)
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] hw/pci/pci.c: Assert that pci_irq_handler() inputs are valid
2021-03-23 16:46 [PATCH] hw/pci/pci.c: Assert that pci_irq_handler() inputs are valid Peter Maydell
2021-03-23 20:47 ` Philippe Mathieu-Daudé
@ 2021-03-24 16:21 ` Isaku Yamahata
1 sibling, 0 replies; 3+ messages in thread
From: Isaku Yamahata @ 2021-03-24 16:21 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-devel, isaku.yamahata, Michael S. Tsirkin
I included (mostly) same patch into my patch series just for patch
completeness.
Please choose whichever you like.
Reviewed-by: Isaku Yamahata <isaku.yamahata@intel.com>
On Tue, Mar 23, 2021 at 04:46:01PM +0000,
Peter Maydell <peter.maydell@linaro.org> wrote:
> pci_irq_handler documents that it must be called with 0 <= irq_num <=
> 3 and level either 0 or 1. Add assertions that the caller has passed
> us in valid arguments.
>
> In particular, if a device model fails to set the PCI_INTERRUPT_PIN
> field in its config space correctly to indicate that it has an
> interrupt, and then tries to raise an interrupt (either by calling
> pci_set_irq(), or by getting a qemu_irq from pci_allocate_irq() and
> then calling qemu_set_irq() on that) we will now diagnose this device
> model bug with an assertion rather than engaging in the undefined
> behaviour of shifting by a negative number.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> hw/pci/pci.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 0eadcdbc9ec..38aefe22ab3 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -1449,6 +1449,9 @@ static void pci_irq_handler(void *opaque, int irq_num, int level)
> PCIDevice *pci_dev = opaque;
> int change;
>
> + assert(irq_num >= 0 && irq_num < PCI_NUM_PINS);
> + assert(level == 0 || level == 1);
> +
> change = level - pci_irq_state(pci_dev, irq_num);
> if (!change)
> return;
> --
> 2.20.1
>
>
--
Isaku Yamahata <isaku.yamahata@gmail.com>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-03-24 16:25 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-23 16:46 [PATCH] hw/pci/pci.c: Assert that pci_irq_handler() inputs are valid Peter Maydell
2021-03-23 20:47 ` Philippe Mathieu-Daudé
2021-03-24 16:21 ` Isaku Yamahata
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).