From cf7723d08da5b371ef8b89a6e4edfaa21f88f03f Mon Sep 17 00:00:00 2001 From: Jon Maloy Date: Tue, 12 Jan 2021 21:01:25 -0500 Subject: memory: clamp cached translation in case it points to an MMIO region RH-Author: Jon Maloy Message-id: <20210112210125.851866-2-jmaloy@redhat.com> Patchwork-id: 100614 O-Subject: [RHEL-AV-8.4.0 qemu-kvm PATCH 1/1] memory: clamp cached translation in case it points to an MMIO region Bugzilla: 1904392 RH-Acked-by: Danilo de Paula RH-Acked-by: Dr. David Alan Gilbert RH-Acked-by: Thomas Huth From: Paolo Bonzini In using the address_space_translate_internal API, address_space_cache_init forgot one piece of advice that can be found in the code for address_space_translate_internal: /* MMIO registers can be expected to perform full-width accesses based only * on their address, without considering adjacent registers that could * decode to completely different MemoryRegions. When such registers * exist (e.g. I/O ports 0xcf8 and 0xcf9 on most PC chipsets), MMIO * regions overlap wildly. For this reason we cannot clamp the accesses * here. * * If the length is small (as is the case for address_space_ldl/stl), * everything works fine. If the incoming length is large, however, * the caller really has to do the clamping through memory_access_size. */ address_space_cache_init is exactly one such case where "the incoming length is large", therefore we need to clamp the resulting length---not to memory_access_size though, since we are not doing an access yet, but to the size of the resulting section. This ensures that subsequent accesses to the cached MemoryRegionSection will be in range. With this patch, the enclosed testcase notices that the used ring does not fit into the MSI-X table and prints a "qemu-system-x86_64: Cannot map used" error. Signed-off-by: Paolo Bonzini (cherry picked from 4bfb024bc76973d40a359476dc0291f46e435442) Signed-off-by: Jon Maloy Signed-off-by: Danilo C. L. de Paula --- softmmu/physmem.c | 10 ++++++++ tests/qtest/fuzz-test.c | 52 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/softmmu/physmem.c b/softmmu/physmem.c index 3027747c03..fb3f276844 100644 --- a/softmmu/physmem.c +++ b/softmmu/physmem.c @@ -3255,6 +3255,7 @@ int64_t address_space_cache_init(MemoryRegionCache *cache, AddressSpaceDispatch *d; hwaddr l; MemoryRegion *mr; + Int128 diff; assert(len > 0); @@ -3263,6 +3264,15 @@ int64_t address_space_cache_init(MemoryRegionCache *cache, d = flatview_to_dispatch(cache->fv); cache->mrs = *address_space_translate_internal(d, addr, &cache->xlat, &l, true); + /* + * cache->xlat is now relative to cache->mrs.mr, not to the section itself. + * Take that into account to compute how many bytes are there between + * cache->xlat and the end of the section. + */ + diff = int128_sub(cache->mrs.size, + int128_make64(cache->xlat - cache->mrs.offset_within_region)); + l = int128_get64(int128_min(diff, int128_make64(l))); + mr = cache->mrs.mr; memory_region_ref(mr); if (memory_access_is_direct(mr, is_write)) { diff --git a/tests/qtest/fuzz-test.c b/tests/qtest/fuzz-test.c index 2692d556d9..99d1a3ee12 100644 --- a/tests/qtest/fuzz-test.c +++ b/tests/qtest/fuzz-test.c @@ -73,6 +73,55 @@ static void test_lp1879531_eth_get_rss_ex_dst_addr(void) qtest_quit(s); } + /* + * Here a MemoryRegionCache pointed to an MMIO region but had a + * larger size than the underlying region. + */ +static void test_mmio_oob_from_memory_region_cache(void) +{ + QTestState *s; + + s = qtest_init("-M pc-q35-5.2 -display none -m 512M " + "-device virtio-scsi,num_queues=8,addr=03.0 "); + + qtest_outl(s, 0xcf8, 0x80001811); + qtest_outb(s, 0xcfc, 0x6e); + qtest_outl(s, 0xcf8, 0x80001824); + qtest_outl(s, 0xcf8, 0x80001813); + qtest_outl(s, 0xcfc, 0xa080000); + qtest_outl(s, 0xcf8, 0x80001802); + qtest_outl(s, 0xcfc, 0x5a175a63); + qtest_outb(s, 0x6e08, 0x9e); + qtest_writeb(s, 0x9f003, 0xff); + qtest_writeb(s, 0x9f004, 0x01); + qtest_writeb(s, 0x9e012, 0x0e); + qtest_writeb(s, 0x9e01b, 0x0e); + qtest_writeb(s, 0x9f006, 0x01); + qtest_writeb(s, 0x9f008, 0x01); + qtest_writeb(s, 0x9f00a, 0x01); + qtest_writeb(s, 0x9f00c, 0x01); + qtest_writeb(s, 0x9f00e, 0x01); + qtest_writeb(s, 0x9f010, 0x01); + qtest_writeb(s, 0x9f012, 0x01); + qtest_writeb(s, 0x9f014, 0x01); + qtest_writeb(s, 0x9f016, 0x01); + qtest_writeb(s, 0x9f018, 0x01); + qtest_writeb(s, 0x9f01a, 0x01); + qtest_writeb(s, 0x9f01c, 0x01); + qtest_writeb(s, 0x9f01e, 0x01); + qtest_writeb(s, 0x9f020, 0x01); + qtest_writeb(s, 0x9f022, 0x01); + qtest_writeb(s, 0x9f024, 0x01); + qtest_writeb(s, 0x9f026, 0x01); + qtest_writeb(s, 0x9f028, 0x01); + qtest_writeb(s, 0x9f02a, 0x01); + qtest_writeb(s, 0x9f02c, 0x01); + qtest_writeb(s, 0x9f02e, 0x01); + qtest_writeb(s, 0x9f030, 0x01); + qtest_outb(s, 0x6e10, 0x00); + qtest_quit(s); +} + int main(int argc, char **argv) { const char *arch = qtest_get_arch(); @@ -86,7 +135,8 @@ int main(int argc, char **argv) test_lp1878642_pci_bus_get_irq_level_assert); qtest_add_func("fuzz/test_lp1879531_eth_get_rss_ex_dst_addr", test_lp1879531_eth_get_rss_ex_dst_addr); - + qtest_add_func("fuzz/test_mmio_oob_from_memory_region_cache", + test_mmio_oob_from_memory_region_cache); } return g_test_run(); -- 2.18.4