Re: [PATCH] softmmu: Use memmove in flatview_write_continue

[Alexander Bulekov]

On 230130 1528, Peter Xu wrote:
> On Mon, Jan 30, 2023 at 03:03:00PM -0500, Alexander Bulekov wrote:
> > On 230130 2251, Akihiko Odaki wrote:
> > > We found a case where the source passed to flatview_write_continue() may
> > > overlap with the destination when fuzzing igb, a new proposed network
> > > device with sanitizers.
> > > 
> > > igb uses pci_dma_map() to get Tx packet, and pci_dma_write() to write Rx
> > > buffer. While pci_dma_write() is usually used to write data from
> > > memory not mapped to the guest, if igb is configured to perform
> > > loopback, the data will be sourced from the guest memory. The source and
> > > destination can overlap and the usage of memcpy() will be invalid in
> > > such a case.
> > > 
> > > While we do not really have to deal with such an invalid request for
> > > igb, detecting the overlap in igb code beforehand requires complex code,
> > > and only covers this specific case. Instead, just replace memcpy() with
> > > memmove() to torelate overlaps. Using memmove() will slightly damage the
> > > performance as it will need to check overlaps before using SIMD
> > > instructions for copying, but the cost should be negiligble, considering
> > > the inherent complexity of flatview_write_continue().
> > > 
> > > The test cases generated by the fuzzer is available at:
> > > 20230129053316.1071513-1-alxndr@bu.edu/">https://patchew.org/QEMU/20230129053316.1071513-1-alxndr@bu.edu/
> > > 
> > > The fixed test case is:
> > > fuzz/crash_47dfe62d9f911bf523ff48cd441b61c0013ed805
> > > 
> > > Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> > 
> > Since this is called fairly often, I went down a rabit hole looking at
> > memmove vs memcpy perf, but It looks like this should not be much of a
> > problem.
> 
> Similar here. I quickly had a look at the dump of memmove() and it seems to
> me it's directly invoking the SIMD instructions.  I'm not sure whether it
> means in many cases the SIMD insts are compatible with overlapped buffers
> already, so it can be mostly the same as memcpy() in modern hardwares.
> 
> > 
> > Acked-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> 
> I received this reply from Alexander Bulekov, but I think this is the
> author's contact?

Sorry about that!
Acked-by: Alexander Bulekov <alxndr@bu.edu>

> 
> Thanks,
> 
> -- 
> Peter Xu
>