lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 15 Nov 2022 11:33:09 +0800
From:   Jason Wang <jasowang@...hat.com>
To:     Eli Cohen <elic@...dia.com>
Cc:     mst@...hat.com, linux-kernel@...r.kernel.org,
        virtualization@...ts.linux-foundation.org, si-wei.liu@...cle.com,
        eperezma@...hat.com, lulu@...hat.com
Subject: Re: [PATH v2 5/8] vdpa/mlx5: Avoid overwriting CVQ iotlb

On Mon, Nov 14, 2022 at 9:18 PM Eli Cohen <elic@...dia.com> wrote:
>
> When qemu uses different address spaces for data and control virtqueues,
> the current code would overwrite the control virtqueue iotlb through the
> dup_iotlb call. Fix this by referring to the address space identifier
> and the group to asid mapping to determine which mapping needs to be
> updated. We also move the address space logic from mlx5 net to core
> directory.
>
> Reported-by: Eugenio PĂ©rez <eperezma@...hat.com>
> Signed-off-by: Eli Cohen <elic@...dia.com>

Acked-by: Jason Wang <jasowang@...hat.com>

Thanks

> ---
>  drivers/vdpa/mlx5/core/mlx5_vdpa.h |  5 +--
>  drivers/vdpa/mlx5/core/mr.c        | 44 ++++++++++++++++-----------
>  drivers/vdpa/mlx5/net/mlx5_vnet.c  | 49 ++++++------------------------
>  3 files changed, 39 insertions(+), 59 deletions(-)
>
> diff --git a/drivers/vdpa/mlx5/core/mlx5_vdpa.h b/drivers/vdpa/mlx5/core/mlx5_vdpa.h
> index 6af9fdbb86b7..058fbe28107e 100644
> --- a/drivers/vdpa/mlx5/core/mlx5_vdpa.h
> +++ b/drivers/vdpa/mlx5/core/mlx5_vdpa.h
> @@ -116,8 +116,9 @@ int mlx5_vdpa_create_mkey(struct mlx5_vdpa_dev *mvdev, u32 *mkey, u32 *in,
>                           int inlen);
>  int mlx5_vdpa_destroy_mkey(struct mlx5_vdpa_dev *mvdev, u32 mkey);
>  int mlx5_vdpa_handle_set_map(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb,
> -                            bool *change_map);
> -int mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb);
> +                            bool *change_map, unsigned int asid);
> +int mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb,
> +                       unsigned int asid);
>  void mlx5_vdpa_destroy_mr(struct mlx5_vdpa_dev *mvdev);
>
>  #define mlx5_vdpa_warn(__dev, format, ...)                                                         \
> diff --git a/drivers/vdpa/mlx5/core/mr.c b/drivers/vdpa/mlx5/core/mr.c
> index a639b9208d41..a4d7ee2339fa 100644
> --- a/drivers/vdpa/mlx5/core/mr.c
> +++ b/drivers/vdpa/mlx5/core/mr.c
> @@ -511,7 +511,8 @@ void mlx5_vdpa_destroy_mr(struct mlx5_vdpa_dev *mvdev)
>         mutex_unlock(&mr->mkey_mtx);
>  }
>
> -static int _mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb)
> +static int _mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev,
> +                               struct vhost_iotlb *iotlb, unsigned int asid)
>  {
>         struct mlx5_vdpa_mr *mr = &mvdev->mr;
>         int err;
> @@ -519,42 +520,49 @@ static int _mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb
>         if (mr->initialized)
>                 return 0;
>
> -       if (iotlb)
> -               err = create_user_mr(mvdev, iotlb);
> -       else
> -               err = create_dma_mr(mvdev, mr);
> +       if (mvdev->group2asid[MLX5_VDPA_DATAVQ_GROUP] == asid) {
> +               if (iotlb)
> +                       err = create_user_mr(mvdev, iotlb);
> +               else
> +                       err = create_dma_mr(mvdev, mr);
>
> -       if (err)
> -               return err;
> +               if (err)
> +                       return err;
> +       }
>
> -       err = dup_iotlb(mvdev, iotlb);
> -       if (err)
> -               goto out_err;
> +       if (mvdev->group2asid[MLX5_VDPA_CVQ_GROUP] == asid) {
> +               err = dup_iotlb(mvdev, iotlb);
> +               if (err)
> +                       goto out_err;
> +       }
>
>         mr->initialized = true;
>         return 0;
>
>  out_err:
> -       if (iotlb)
> -               destroy_user_mr(mvdev, mr);
> -       else
> -               destroy_dma_mr(mvdev, mr);
> +       if (mvdev->group2asid[MLX5_VDPA_DATAVQ_GROUP] == asid) {
> +               if (iotlb)
> +                       destroy_user_mr(mvdev, mr);
> +               else
> +                       destroy_dma_mr(mvdev, mr);
> +       }
>
>         return err;
>  }
>
> -int mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb)
> +int mlx5_vdpa_create_mr(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb,
> +                       unsigned int asid)
>  {
>         int err;
>
>         mutex_lock(&mvdev->mr.mkey_mtx);
> -       err = _mlx5_vdpa_create_mr(mvdev, iotlb);
> +       err = _mlx5_vdpa_create_mr(mvdev, iotlb, asid);
>         mutex_unlock(&mvdev->mr.mkey_mtx);
>         return err;
>  }
>
>  int mlx5_vdpa_handle_set_map(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb,
> -                            bool *change_map)
> +                            bool *change_map, unsigned int asid)
>  {
>         struct mlx5_vdpa_mr *mr = &mvdev->mr;
>         int err = 0;
> @@ -566,7 +574,7 @@ int mlx5_vdpa_handle_set_map(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *io
>                 *change_map = true;
>         }
>         if (!*change_map)
> -               err = _mlx5_vdpa_create_mr(mvdev, iotlb);
> +               err = _mlx5_vdpa_create_mr(mvdev, iotlb, asid);
>         mutex_unlock(&mr->mkey_mtx);
>
>         return err;
> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> index 98dd8ce8af26..3a6dbbc6440d 100644
> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> @@ -2394,7 +2394,8 @@ static void restore_channels_info(struct mlx5_vdpa_net *ndev)
>         }
>  }
>
> -static int mlx5_vdpa_change_map(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb)
> +static int mlx5_vdpa_change_map(struct mlx5_vdpa_dev *mvdev,
> +                               struct vhost_iotlb *iotlb, unsigned int asid)
>  {
>         struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev);
>         int err;
> @@ -2406,7 +2407,7 @@ static int mlx5_vdpa_change_map(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb
>
>         teardown_driver(ndev);
>         mlx5_vdpa_destroy_mr(mvdev);
> -       err = mlx5_vdpa_create_mr(mvdev, iotlb);
> +       err = mlx5_vdpa_create_mr(mvdev, iotlb, asid);
>         if (err)
>                 goto err_mr;
>
> @@ -2587,7 +2588,7 @@ static int mlx5_vdpa_reset(struct vdpa_device *vdev)
>         ++mvdev->generation;
>
>         if (MLX5_CAP_GEN(mvdev->mdev, umem_uid_0)) {
> -               if (mlx5_vdpa_create_mr(mvdev, NULL))
> +               if (mlx5_vdpa_create_mr(mvdev, NULL, 0))
>                         mlx5_vdpa_warn(mvdev, "create MR failed\n");
>         }
>         up_write(&ndev->reslock);
> @@ -2623,41 +2624,20 @@ static u32 mlx5_vdpa_get_generation(struct vdpa_device *vdev)
>         return mvdev->generation;
>  }
>
> -static int set_map_control(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb)
> -{
> -       u64 start = 0ULL, last = 0ULL - 1;
> -       struct vhost_iotlb_map *map;
> -       int err = 0;
> -
> -       spin_lock(&mvdev->cvq.iommu_lock);
> -       vhost_iotlb_reset(mvdev->cvq.iotlb);
> -
> -       for (map = vhost_iotlb_itree_first(iotlb, start, last); map;
> -            map = vhost_iotlb_itree_next(map, start, last)) {
> -               err = vhost_iotlb_add_range(mvdev->cvq.iotlb, map->start,
> -                                           map->last, map->addr, map->perm);
> -               if (err)
> -                       goto out;
> -       }
> -
> -out:
> -       spin_unlock(&mvdev->cvq.iommu_lock);
> -       return err;
> -}
> -
> -static int set_map_data(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb)
> +static int set_map_data(struct mlx5_vdpa_dev *mvdev, struct vhost_iotlb *iotlb,
> +                       unsigned int asid)
>  {
>         bool change_map;
>         int err;
>
> -       err = mlx5_vdpa_handle_set_map(mvdev, iotlb, &change_map);
> +       err = mlx5_vdpa_handle_set_map(mvdev, iotlb, &change_map, asid);
>         if (err) {
>                 mlx5_vdpa_warn(mvdev, "set map failed(%d)\n", err);
>                 return err;
>         }
>
>         if (change_map)
> -               err = mlx5_vdpa_change_map(mvdev, iotlb);
> +               err = mlx5_vdpa_change_map(mvdev, iotlb, asid);
>
>         return err;
>  }
> @@ -2670,16 +2650,7 @@ static int mlx5_vdpa_set_map(struct vdpa_device *vdev, unsigned int asid,
>         int err = -EINVAL;
>
>         down_write(&ndev->reslock);
> -       if (mvdev->group2asid[MLX5_VDPA_DATAVQ_GROUP] == asid) {
> -               err = set_map_data(mvdev, iotlb);
> -               if (err)
> -                       goto out;
> -       }
> -
> -       if (mvdev->group2asid[MLX5_VDPA_CVQ_GROUP] == asid)
> -               err = set_map_control(mvdev, iotlb);
> -
> -out:
> +       err = set_map_data(mvdev, iotlb, asid);
>         up_write(&ndev->reslock);
>         return err;
>  }
> @@ -3182,7 +3153,7 @@ static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name,
>                 goto err_mpfs;
>
>         if (MLX5_CAP_GEN(mvdev->mdev, umem_uid_0)) {
> -               err = mlx5_vdpa_create_mr(mvdev, NULL);
> +               err = mlx5_vdpa_create_mr(mvdev, NULL, 0);
>                 if (err)
>                         goto err_res;
>         }
> --
> 2.38.1
>

Powered by blists - more mailing lists