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:
> https://patchew.org/QEMU/20230129053316.1071513-1-alx...@bu.edu/
>
> The fixed test case is:
> fuzz/crash_47dfe62d9f911bf523ff48cd441b61c0013ed805
>
> Signed-off-by: Akihiko Odaki <akihiko.od...@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.
Acked-by: Akihiko Odaki <akihiko.od...@daynix.com>
Thank you
> Signed-off-by: Akihiko Odaki <akihiko.od...@daynix.com>
> ---
> softmmu/physmem.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/softmmu/physmem.c b/softmmu/physmem.c
> index cb998cdf23..3cd27b1c9d 100644
> --- a/softmmu/physmem.c
> +++ b/softmmu/physmem.c
> @@ -2828,7 +2828,7 @@ static MemTxResult flatview_write_continue(FlatView
> *fv, hwaddr addr,
> } else {
> /* RAM case */
> ram_ptr = qemu_ram_ptr_length(mr->ram_block, addr1, &l, false);
> - memcpy(ram_ptr, buf, l);
> + memmove(ram_ptr, buf, l);
> invalidate_and_set_dirty(mr, addr1, l);
> }
>
> --
> 2.39.1
>
