Re: [Qemu-devel] [PATCH] exec: Fix section_covers_addr() for sections with non-zero offset

[BALATON Zoltan]

On Fri, 27 Oct 2017, BALATON Zoltan wrote:

On Sat, 21 Oct 2017, BALATON Zoltan wrote:

When a section with non-0 offset_within_region field is tested to
cover an address the offset should be taken into account as well.

This fixes a crash caused by picking the wrong memory region in
address_space_lookup_region seen with client code accessing a device
model that uses alias memory regions.

Signed-off-by: BALATON Zoltan <bala...@eik.bme.hu>
---

Ping? http://patchwork.ozlabs.org/project/qemu-devel/list/?series=9457

Ping!

This seems to fix the problem described in
http://lists.nongnu.org/archive/html/qemu-devel/2017-10/msg03356.html
but I'm not completely sure about it. This seems to be introduced in
729633c exec: Introduce AddressSpaceDispatch.mru_section and the patch
before that which split off section_covers_addr from phys_page_find so
this patch also changes that caller. Is that OK to do? It appears to
work but I don't know this part of QEMU.

Also the bug seems to be caused by section_covers_addr accepting
sii3112.bar5 when that's the mru_section instead of picking
sii3112.bar0 (which it picks when going through phys_page_find) when
client code is accessing 0xc08001006 from this address map (full
address map is at above URL):

address-space: memory

0000000c08000000-0000000c0800ffff (prio 0, i/o): alias isa_mmio @io 0000000000000000-000000000000ffff

address-space: I/O
 0000000000000000-000000000000ffff (prio 0, i/o): io

0000000000001000-0000000000001007 (prio 1, i/o): alias sii3112.bar0 @sii3112.bar5 0000000000000080-0000000000000087 0000000000001008-000000000000100b (prio 1, i/o): alias sii3112.bar1 @sii3112.bar5 0000000000000088-000000000000008b 0000000000001010-0000000000001017 (prio 1, i/o): alias sii3112.bar2 @sii3112.bar5 00000000000000c0-00000000000000c7 0000000000001018-000000000000101b (prio 1, i/o): alias sii3112.bar3 @sii3112.bar5 00000000000000c8-00000000000000cb 0000000000001020-000000000000102f (prio 1, i/o): alias sii3112.bar4 @sii3112.bar5 0000000000000000-000000000000000f

which this patch fixes but would the same problem happen if the
mru_section is bar5 but bar4 is accessed? I could not reproduce that
case but then the offset is 0 but in this case the address would be
above 0xc08001020 and size is 0xf so they probably won't match. But
this is only because of the size of the region. Could that mean the
bug is caused by something else and should be fixed elsewhere?

---
exec.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/exec.c b/exec.c
index db5ae23..a915817 100644
--- a/exec.c
+++ b/exec.c

@@ -370,7 +370,8 @@ static inline bool section_covers_addr(const MemoryRegionSection *section,

     * the section must cover the entire address space.
     */
    return int128_gethi(section->size) ||
-           range_covers_byte(section->offset_within_address_space,
+           range_covers_byte(section->offset_within_address_space +
+                             section->offset_within_region,
                             int128_getlo(section->size), addr);
}