From: Jan Kiszka
Sent: Tue 9/20/2011 3:41 PM
To: Alan Amaral
Cc: Richard Henderson; address@hidden
Subject: Re: pci_change_irq_level is broken...
> On 2011-09-20 21:19, Alan Amaral wrote:
> > QEMU emulator version 0.14.50, Copyright (c) 2003-2008 Fabrice Bellard
>
> (That's an ambitious development version.)
It's what we're using.
> >
> > You are correct, it's not hardcoded to 4. However, when it's allocated the number of elements IS 4. Also,
> > there's a comment just above pci_set_irq which says:
> >
> > /* 0 <= irq_num <= 3. level must be 0 or 1 */
> > static void pci_set_irq(void *opaque, int irq_num, int level)
> >
> > so, that implies to me that it's probably always 4... Sorry for the confusion.
>
> Assuming you look at PIIX3: Yes, it allocates 4 IRQs - but only returns
> 0..3 via pci_slot_get_pirq. Xen uses some more, but also looks safe.
We are running under Xen and in this case it is using PIIX_NUM_PIRQS,
which is 4, as the last arg to pci_bus_irqs().
PCIBus *i440fx_xen_init(PCII440FXState **pi440fx_state, int *piix3_devfn,
qemu_irq *pic, ram_addr_t ram_size)
{
PCIBus *b;
b = i440fx_common_init("i440FX-xen", pi440fx_state, piix3_devfn, pic, ram_size);
pci_bus_irqs(b, xen_piix3_set_irq, xen_pci_slot_get_pirq,
(*pi440fx_state)->piix3, PIIX_NUM_PIRQS);
return b;
}
Also, since we are using xen, xen_pci_slot_get_pirq is being used, which always returns
something >= 4 (at least when pci_dev->devfn > 0). Here's the code:
int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
{
return irq_num + ((pci_dev->devfn >> 3) << 2);
}
In the case I'm seeing devfn == 9. I'm in gdb now, and at a breakpoint at the error condition.
The call is:
Breakpoint 1, pci_change_irq_level (pci_dev=0x1c3a730, irq_num=0, change=0)
(gdb) p *pci_dev
$1 = {qdev = {id = 0x0, state = DEV_STATE_INITIALIZED, opts = 0x0,
hotplugged = 0, info = 0xa08c00, parent_bus = 0x1af5700, num_gpio_out = 0,
gpio_out = 0x0, num_gpio_in = 0, gpio_in = 0x0, child_bus = {
lh_first = 0x1c3b0c8}, num_child_bus = 2, sibling = {
le_next = 0x1c37440, le_prev = 0x1c9dcb0}, instance_id_alias = -1,
alias_required_for_version = 0}, config = 0x1c3b8e0 "\206\200\020p",
cmask = 0x1c3b9f0 "\377\377\377\377", wmask = 0x1c3bb00 "",
w1cmask = 0x1c3bc10 "", used = 0x1c3bd20 "", bus = 0x1af5700,
devfn = 9,
name = "piix3-ide", '\000' <repeats 54 times>, io_regions = {{addr = 0,
size = 0, filtered_size = 0, type = 0 '\000', map_func = 0,
ram_addr = 0}, {addr = 0, size = 0, filtered_size = 0, type = 0 '\000',
map_func = 0, ram_addr = 0}, {addr = 0, size = 0, filtered_size = 0,
type = 0 '\000', map_func = 0, ram_addr = 0}, {addr = 0, size = 0,
filtered_size = 0, type = 0 '\000', map_func = 0, ram_addr = 0}, {
addr = 18446744073709551615, size = 16, filtered_size = 16,
type = 1 '\001', map_func = 0x60095c <bmdma_map>, ram_addr = 16}, {
addr = 0, size = 0, filtered_size = 0, type = 0 '\000', map_func = 0,
ram_addr = 0}, {addr = 0, size = 0, filtered_size = 0, type = 0 '\000',
map_func = 0, ram_addr = 0}},
config_read = 0x5be0c9 <pci_default_read_config>,
config_write = 0x5be192 <pci_default_write_config>, irq = 0x1c3be30,
irq_state = 0 '\000', cap_present = 16, msix_cap = 0 '\000',
msix_entries_nr = 0, msix_table_page = 0x0, msix_mmio_index = 0,
msix_entry_used = 0x0, msix_bar_size = 0, version_id = 2,
msi_cap = 0 '\000', exp = {exp_cap = 0 '\000', hpev_intx = 0,
hpev_notified = false, aer_cap = 0, aer_log = {log_num = 0, log_max = 0,
log = 0x0}, aer_intx = 0}, romfile = 0x0, rom_offset = 0, rom_bar = 1}
(gdb) p *bus
$2 = {qbus = {parent = 0x1af41b0, info = 0x9e4e60, name = 0x1736910 "pci.0",
allow_hotplug = 1, qdev_allocated = 1, children = {lh_first = 0x1cd3520},
sibling = {le_next = 0x0, le_prev = 0x1af4200}}, devfn_min = 0 '\000',
set_irq = 0x62e600 <xen_piix3_set_irq>,
map_irq = 0x62e5b2 <xen_pci_slot_get_pirq>, hotplug = 0x5d71b8 <piix4_device_hotplug>, hotplug_qdev = 0x1cd1600,
irq_opaque = 0x1af6fd0, devices = {0x1af6150, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
0x0, 0x1af6fd0, 0x1c3a730, 0x1cd09c0, 0x1cd1600, 0x0, 0x0, 0x0, 0x0,
0x1af7be0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1c37440, 0x0, 0x0, 0x0,
0x0, 0x0, 0x0, 0x0, 0x1cbe010, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
0x1c9dc50, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1cd3520,
0x0 <repeats 207 times>}, parent_dev = 0x0, mem_base = 0, child = {
lh_first = 0x0}, sibling = {le_next = 0x0, le_prev = 0x0},
nirq = 4,
irq_count = 0x1af7ab0}
(gdb) p bus->nirq
$3 = 4
(gdb) p pci_dev->devfn
$4 = 9
(gdb) p bus->map_irq
$5 = (pci_map_irq_fn) 0x62e5b2 <xen_pci_slot_get_pirq>
After the first call to map_irq irq_num is changed to 4:
(gdb) n
119 bus = pci_dev->bus;
(gdb)
120 irq_num = bus->map_irq(pci_dev, irq_num);
(gdb)
121 if (bus->set_irq)
(gdb) p irq_num
$6 = 4
(gdb) p bus->set_irq
$7 = (pci_set_irq_fn) 0x62e600 <xen_piix3_set_irq>
(gdb) n
125 bus->irq_count[irq_num] += change;
(gdb) p irq_num
$8 = 4
(gdb) p bus->nirq
$9 = 4
(gdb) whatis bus->irq_count[0]
type = int
(gdb) p sizeof(bus->irq_count[0])
$10 = 4
(gdb)
This all clearly shows that the irq_count array has 4 elements and the code
is trying to read and write irq_count[4], which is outside of the malloc'd block.
> Can you provide a backtrace where irq_num gets larger than 3 and writes
> beyond the end of irq_count? Do you have private patches in your tree?
Backtrace is below. Oh, yes, we're using private patches, but I don't believe
that this code has been patched at all.
> Jan
>
Here's a stack trace from valgrind. You'll note that the allocated block size is 16,
which is 4 ints, and I verified above in gdb that bus->irq_nirq was 4.