Re: [PATCH V2] virtio-net: check the existence of peer before accessing vDPA config

[Michael S. Tsirkin]

On Mon, Jul 27, 2020 at 08:51:50PM +0800, Jason Wang wrote:
> We try to check whether a peer is VDPA in order to get config from
> there - with no peer, this leads to a NULL
> pointer dereference. Add a check before trying to access the peer
> type. No peer means not VDPA.
> 
> Fixes: 108a64818e69b ("vhost-vdpa: introduce vhost-vdpa backend")
> Cc: Cindy Lu <lulu@redhat.com>
> Tested-by: Cornelia Huck <cohuck@redhat.com>
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
> Changes from V1:
> - Tweak the log
> - Add the comment in the code
> ---
>  hw/net/virtio-net.c | 30 +++++++++++++++++++-----------
>  1 file changed, 19 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 4895af1cbe..a1fe9e9285 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -125,6 +125,7 @@ static void virtio_net_get_config(VirtIODevice *vdev, 
> uint8_t *config)
>  {
>      VirtIONet *n = VIRTIO_NET(vdev);
>      struct virtio_net_config netcfg;
> +    NetClientState *nc = qemu_get_queue(n->nic);
>  
>      int ret = 0;
>      memset(&netcfg, 0 , sizeof(struct virtio_net_config));
> @@ -142,13 +143,16 @@ static void virtio_net_get_config(VirtIODevice *vdev, 
> uint8_t *config)
>                   VIRTIO_NET_RSS_SUPPORTED_HASHES);
>      memcpy(config, &netcfg, n->config_size);
>  
> -    NetClientState *nc = qemu_get_queue(n->nic);
> -    if (nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
> +    /*
> +     * Is this VDPA? No peer means not VDPA: there's no way to
> +     * disconnect/reconnect a VDPA peer.
> +     */
> +    if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
>          ret = vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t 
> *)&netcfg,
> -                             n->config_size);
> -    if (ret != -1) {
> -        memcpy(config, &netcfg, n->config_size);
> -    }
> +                                   n->config_size);
> +        if (ret != -1) {
> +            memcpy(config, &netcfg, n->config_size);
> +        }
>      }
>  }
>  
> @@ -156,6 +160,7 @@ static void virtio_net_set_config(VirtIODevice *vdev, 
> const uint8_t *config)
>  {
>      VirtIONet *n = VIRTIO_NET(vdev);
>      struct virtio_net_config netcfg = {};
> +    NetClientState *nc = qemu_get_queue(n->nic);
>  
>      memcpy(&netcfg, config, n->config_size);
>  
> @@ -166,11 +171,14 @@ static void virtio_net_set_config(VirtIODevice *vdev, 
> const uint8_t *config)
>          qemu_format_nic_info_str(qemu_get_queue(n->nic), n->mac);
>      }
>  
> -    NetClientState *nc = qemu_get_queue(n->nic);
> -    if (nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
> -        vhost_net_set_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg,
> -                               0, n->config_size,
> -                        VHOST_SET_CONFIG_TYPE_MASTER);
> +    /*
> +     * Is this VDPA? No peer means not VDPA: there's no way to
> +     * disconnect/reconnect a VDPA peer.
> +     */
> +    if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
> +        vhost_net_set_config(get_vhost_net(nc->peer),
> +                             (uint8_t *)&netcfg, 0, n->config_size,
> +                             VHOST_SET_CONFIG_TYPE_MASTER);
>        }
>  }


Do we want a helper for this? E.g. qemu_test_peer_type?

Anyway:

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

> -- 
> 2.20.1