All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] exec: Fix section_covers_addr() for sections with non-zero offset
@ 2017-10-21 11:24 BALATON Zoltan
  2017-10-21 12:57 ` no-reply
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: BALATON Zoltan @ 2017-10-21 11:24 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, Peter Crosthwaite, Richard Henderson, Fam Zheng,
	Peter Maydell

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 <balaton@eik.bme.hu>
---
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);
 }
 
-- 
2.7.6

^ permalink raw reply related	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-11-14 23:03 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-21 11:24 [Qemu-devel] [PATCH] exec: Fix section_covers_addr() for sections with non-zero offset BALATON Zoltan
2017-10-21 12:57 ` no-reply
2017-10-26 22:21 ` BALATON Zoltan
2017-11-14 17:36   ` BALATON Zoltan
2017-11-14 18:04 ` Paolo Bonzini
2017-11-14 23:03   ` BALATON Zoltan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.