[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PULL 19/45] memory: clamp cached translation in case it points to a
From: |
Michael S. Tsirkin |
Subject: |
Re: [PULL 19/45] memory: clamp cached translation in case it points to an MMIO region |
Date: |
Wed, 13 Jan 2021 08:27:49 -0500 |
On Tue, Dec 15, 2020 at 12:54:19PM -0500, Paolo Bonzini wrote:
> 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 <pbonzini@redhat.com>
Cc: qemu-stable@nongnu.org
> ---
> softmmu/physmem.c | 10 ++++++++
> tests/qtest/fuzz-test.c | 51 +++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 61 insertions(+)
>
> diff --git a/softmmu/physmem.c b/softmmu/physmem.c
> index 3027747c03..2cd1de4a2c 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 87b72307a5..cdb1100a0b 100644
> --- a/tests/qtest/fuzz-test.c
> +++ b/tests/qtest/fuzz-test.c
> @@ -48,6 +48,55 @@ static void
> test_lp1878642_pci_bus_get_irq_level_assert(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();
> @@ -59,6 +108,8 @@ int main(int argc, char **argv)
> test_lp1878263_megasas_zero_iov_cnt);
> qtest_add_func("fuzz/test_lp1878642_pci_bus_get_irq_level_assert",
> test_lp1878642_pci_bus_get_irq_level_assert);
> + qtest_add_func("fuzz/test_mmio_oob_from_memory_region_cache",
> + test_mmio_oob_from_memory_region_cache);
> }
>
> return g_test_run();
> --
> 2.26.2
>
>
>
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [PULL 19/45] memory: clamp cached translation in case it points to an MMIO region,
Michael S. Tsirkin <=