Re: [PATCH] vfio/pci: Use defined memcpy() behavior

[Longpeng (Mike, Cloud Infrastructure Service Product Dept.)]

On 2020/3/11 1:15, Alex Williamson wrote:
> vfio_rom_read() relies on memcpy() doing the logically correct thing,
> ie. safely copying zero bytes from a NULL pointer when rom_size is
> zero, rather than the spec definition, which is undefined when the
> source or target pointers are NULL.  Resolve this by wrapping the
> call in the condition expressed previously by the ternary.
> 
> Additionally, we still use @val to fill data based on the provided
> @size regardless of mempcy(), so we should initialize @val rather
> than @data.
> 
> Reported-by: Longpeng <longpe...@huawei.com>
> Reported-by: Laszlo Ersek <ler...@redhat.com>
> Signed-off-by: Alex Williamson <alex.william...@redhat.com>
> ---
>  hw/vfio/pci.c |    9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 5e75a95129ac..b0799cdc28ad 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -859,16 +859,17 @@ static uint64_t vfio_rom_read(void *opaque, hwaddr 
> addr, unsigned size)
>          uint16_t word;
>          uint32_t dword;
>          uint64_t qword;
> -    } val;
> -    uint64_t data = 0;
> +    } val = { 0 };
> +    uint64_t data;
>  
'val.qword' won't be used, maybe we could drop it silently.

Reviewed-by: Longpeng <longpe...@huawei.com>

>      /* Load the ROM lazily when the guest tries to read it */
>      if (unlikely(!vdev->rom && !vdev->rom_read_failed)) {
>          vfio_pci_load_rom(vdev);
>      }
>  
> -    memcpy(&val, vdev->rom + addr,
> -           (addr < vdev->rom_size) ? MIN(size, vdev->rom_size - addr) : 0);
> +    if (addr < vdev->rom_size) {
> +        memcpy(&val, vdev->rom + addr, MIN(size, vdev->rom_size - addr));
> +    }
>  
>      switch (size) {
>      case 1:
> 
> .
> 
---
Regards,
Longpeng(Mike)