Re: fuzz: fuzz_dma_read_cb() may overlap with MMIO regions

[Alexander Bulekov]

On 210823 2034, Qiuhao Li wrote:
> I think the check in fuzz_dma_read_cb() is buggy because it doesn't
> consider when the write address is not in the mmio region but can
> overlap. For example, the mmio region is 0xe0000000 to 0xe0001000, and
> the write address is 0xdffff000 (not ram) and length is 0x2000. In this
> case, the address_space_translate() will return the sparse_mem_mr we
> created, thus bypassing the check and call qtest_memwrite().
> 
> Perhaps we need more detailed checks to ensure that the entire write
> operation occurs in the ram or won't overlap with mmio regions. What do
> you think?
> 
> 

Good catch. I think this will fix that:
https://lore.kernel.org/qemu-devel/20210713150037.9297-2-alxndr@bu.edu/

I mentioned that there were some fixes waiting for the 6.1 release, but
didn't realize you were talking about what seems to be the same issue.
-Alex

> mr1 = address_space_translate(first_cpu->as,
>                               addr, &addr1, &l, true,
>                               MEMTXATTRS_UNSPECIFIED);
> 
> if (!(memory_region_is_ram(mr1) ||
>       memory_region_is_romd(mr1)) && mr1 != sparse_mem_mr) {
>     l = memory_access_size(mr1, l, addr1);
> } else {
>     /* ROM/RAM case */
>     // mr1 == sparse_mem_mr but it's not RAM or ROM <--
>     // May overlap with mmio regions                <--
>     ...
>     qtest_memwrite(qts_global, addr, buf, l);
> 
> 
> Thanks.
>   Qiuhao Li
> 
> On Mon, 2021-08-23 at 04:14 -0400, Alexander Bulekov wrote:
> > I'm not sure I understand. We try to avoid writing to MMIO regions in
> > fuzz_dma_read_cb to avoid such false-positives. E.g. that's why we have
> > code to do address_space_translate and manually walk the AddressSpace
> > and verify that we are writing to RAM, before doing the actual
> > qtest_memwrite. There is a fix to that code that need to be applied,
> > but
> > those have to wait for the 6.1 release. BTW, since this is about the
> > generic-fuzzer rather than this bug, I cc-ed qemu-devel. Let's continue
> > the discussion there.
> > 
> > -Alex
> > 
> > On 210823 0132, 李秋豪 (@QiuhaoLi) wrote:
> > > 
> > > 
> > > 
> > > 李秋豪 commented on a discussion:
> > > https://gitlab.com/qemu-project/qemu/-/issues/541#note_657305687
> > > 
> > > Ok, I add a reply to my report about #540 and #541.
> > > 
> > > Btw, it suddenly occurred to me that our generic-fuzzer can also make
> > > reentry issues. For example, a device tries to read from a mmio
> > > region while being fuzzed, but the fuzz_dma_read_cb() will write to
> > > that region, thus leading to positive-false reentry issues. In short,
> > > we change a read action to write. Should we add checks?
> > > 
> > > -- 
> > > Reply to this email directly or view it on GitLab:
> > > https://gitlab.com/qemu-project/qemu/-/issues/541#note_657305687
> > > You're receiving this email because of your account on gitlab.com.
> > > 
> > > 
> > 
> 
>