Jason Wang
2021-Aug-19 03:40 UTC
[PATCH v2 4/6] vdpa/mlx5: Ensure valid indices are provided
? 2021/8/17 ??2:02, Eli Cohen ??:> Following patches add control virtuqeue and multiqueue support. We want > to verify that the index value to callbacks referencing a virtqueue is > valid. > > The logic defining valid indices is as follows: > CVQ clear: 0 and 1. > CVQ set, MQ clear: 0, 1 and 2 > CVQ set, MQ set: 0..nvq where nvq is whatever provided to > _vdpa_register_device() > > Signed-off-by: Eli Cohen <elic at nvidia.com> > --- > drivers/vdpa/mlx5/core/mlx5_vdpa.h | 1 + > drivers/vdpa/mlx5/net/mlx5_vnet.c | 48 ++++++++++++++++++++++++++++++ > 2 files changed, 49 insertions(+) > > diff --git a/drivers/vdpa/mlx5/core/mlx5_vdpa.h b/drivers/vdpa/mlx5/core/mlx5_vdpa.h > index 8d0a6f2cb3f0..41b20855ed31 100644 > --- a/drivers/vdpa/mlx5/core/mlx5_vdpa.h > +++ b/drivers/vdpa/mlx5/core/mlx5_vdpa.h > @@ -56,6 +56,7 @@ struct mlx5_vdpa_dev { > u64 actual_features; > u8 status; > u32 max_vqs; > + u16 max_idx; > u32 generation; > > struct mlx5_vdpa_mr mr; > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c > index 222ddfbde116..0fe7cd370e4b 100644 > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c > @@ -45,6 +45,8 @@ MODULE_LICENSE("Dual BSD/GPL"); > (VIRTIO_CONFIG_S_ACKNOWLEDGE | VIRTIO_CONFIG_S_DRIVER | VIRTIO_CONFIG_S_DRIVER_OK | \ > VIRTIO_CONFIG_S_FEATURES_OK | VIRTIO_CONFIG_S_NEEDS_RESET | VIRTIO_CONFIG_S_FAILED) > > +#define MLX5_FEATURE(_mvdev, _feature) (!!((_mvdev)->actual_features & BIT_ULL(_feature))) > + > struct mlx5_vdpa_net_resources { > u32 tisn; > u32 tdn; > @@ -133,6 +135,14 @@ struct mlx5_vdpa_virtqueue { > */ > #define MLX5_MAX_SUPPORTED_VQS 16 > > +static bool is_index_valid(struct mlx5_vdpa_dev *mvdev, u16 idx) > +{ > + if (unlikely(idx > mvdev->max_idx)) > + return false; > + > + return true; > +} > + > struct mlx5_vdpa_net { > struct mlx5_vdpa_dev mvdev; > struct mlx5_vdpa_net_resources res; > @@ -1355,6 +1365,9 @@ static void mlx5_vdpa_kick_vq(struct vdpa_device *vdev, u16 idx) > struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev); > struct mlx5_vdpa_virtqueue *mvq = &ndev->vqs[idx]; > > + if (!is_index_valid(mvdev, idx)) > + return; > + > if (unlikely(!mvq->ready)) > return; > > @@ -1368,6 +1381,9 @@ static int mlx5_vdpa_set_vq_address(struct vdpa_device *vdev, u16 idx, u64 desc_ > struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev); > struct mlx5_vdpa_virtqueue *mvq = &ndev->vqs[idx]; > > + if (!is_index_valid(mvdev, idx)) > + return -EINVAL; > + > mvq->desc_addr = desc_area; > mvq->device_addr = device_area; > mvq->driver_addr = driver_area; > @@ -1380,6 +1396,9 @@ static void mlx5_vdpa_set_vq_num(struct vdpa_device *vdev, u16 idx, u32 num) > struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev); > struct mlx5_vdpa_virtqueue *mvq; > > + if (!is_index_valid(mvdev, idx)) > + return; > + > mvq = &ndev->vqs[idx]; > mvq->num_ent = num; > } > @@ -1398,6 +1417,9 @@ static void mlx5_vdpa_set_vq_ready(struct vdpa_device *vdev, u16 idx, bool ready > struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev); > struct mlx5_vdpa_virtqueue *mvq = &ndev->vqs[idx]; > > + if (!is_index_valid(mvdev, idx)) > + return; > + > if (!ready) > suspend_vq(ndev, mvq); > > @@ -1410,6 +1432,9 @@ static bool mlx5_vdpa_get_vq_ready(struct vdpa_device *vdev, u16 idx) > struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev); > struct mlx5_vdpa_virtqueue *mvq = &ndev->vqs[idx]; > > + if (!is_index_valid(mvdev, idx)) > + return false; > + > return mvq->ready; > } > > @@ -1420,6 +1445,9 @@ static int mlx5_vdpa_set_vq_state(struct vdpa_device *vdev, u16 idx, > struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev); > struct mlx5_vdpa_virtqueue *mvq = &ndev->vqs[idx]; > > + if (!is_index_valid(mvdev, idx)) > + return -EINVAL; > + > if (mvq->fw_state == MLX5_VIRTIO_NET_Q_OBJECT_STATE_RDY) { > mlx5_vdpa_warn(mvdev, "can't modify available index\n"); > return -EINVAL; > @@ -1438,6 +1466,9 @@ static int mlx5_vdpa_get_vq_state(struct vdpa_device *vdev, u16 idx, struct vdpa > struct mlx5_virtq_attr attr; > int err; > > + if (!is_index_valid(mvdev, idx)) > + return -EINVAL; > + > /* If the virtq object was destroyed, use the value saved at > * the last minute of suspend_vq. This caters for userspace > * that cares about emulating the index after vq is stopped. > @@ -1557,6 +1588,18 @@ static __virtio16 cpu_to_mlx5vdpa16(struct mlx5_vdpa_dev *mvdev, u16 val) > return __cpu_to_virtio16(mlx5_vdpa_is_little_endian(mvdev), val); > } > > +static void update_cvq_info(struct mlx5_vdpa_dev *mvdev) > +{ > + if (MLX5_FEATURE(mvdev, VIRTIO_NET_F_CTRL_VQ)) { > + if (MLX5_FEATURE(mvdev, VIRTIO_NET_F_MQ)) > + mvdev->max_idx = mvdev->max_vqs; > + else > + mvdev->max_idx = 2; > + } else { > + mvdev->max_idx = 1; > + } > +}Nit: it might be better to add a comment to explain the logic here. E.g we know index 0 and 1 should always valid. Other than this: Acked-by: Jason Wang <jasowang at redhat.com>> + > static int mlx5_vdpa_set_features(struct vdpa_device *vdev, u64 features) > { > struct mlx5_vdpa_dev *mvdev = to_mvdev(vdev); > @@ -1572,6 +1615,7 @@ static int mlx5_vdpa_set_features(struct vdpa_device *vdev, u64 features) > ndev->mvdev.actual_features = features & ndev->mvdev.mlx_features; > ndev->config.mtu = cpu_to_mlx5vdpa16(mvdev, ndev->mtu); > ndev->config.status |= cpu_to_mlx5vdpa16(mvdev, VIRTIO_NET_S_LINK_UP); > + update_cvq_info(mvdev); > return err; > } > > @@ -1792,6 +1836,7 @@ static void mlx5_vdpa_set_status(struct vdpa_device *vdev, u8 status) > mlx5_vdpa_destroy_mr(&ndev->mvdev); > ndev->mvdev.status = 0; > ndev->mvdev.mlx_features = 0; > + ndev->mvdev.actual_features = 0; > ++mvdev->generation; > if (MLX5_CAP_GEN(mvdev->mdev, umem_uid_0)) { > if (mlx5_vdpa_create_mr(mvdev, NULL)) > @@ -1892,6 +1937,9 @@ static struct vdpa_notification_area mlx5_get_vq_notification(struct vdpa_device > struct mlx5_vdpa_net *ndev; > phys_addr_t addr; > > + if (!is_index_valid(mvdev, idx)) > + return ret; > + > /* If SF BAR size is smaller than PAGE_SIZE, do not use direct > * notification to avoid the risk of mapping pages that contain BAR of more > * than one SF