Messages in this thread |  | | Date | Tue, 9 Feb 2021 09:58:45 +0100 | From | Stefano Garzarella <> | Subject | Re: [PATCH] vdpa/mlx5: fix param validation in mlx5_vdpa_get_config() |
| |
On Tue, Feb 09, 2021 at 11:24:03AM +0800, Jason Wang wrote: > >On 2021/2/9 上午2:38, Michael S. Tsirkin wrote: >>On Mon, Feb 08, 2021 at 05:17:41PM +0100, Stefano Garzarella wrote: >>>It's legal to have 'offset + len' equal to >>>sizeof(struct virtio_net_config), since 'ndev->config' is a >>>'struct virtio_net_config', so we can safely copy its content under >>>this condition. >>> >>>Fixes: 1a86b377aa21 ("vdpa/mlx5: Add VDPA driver for supported mlx5 devices") >>>Cc: stable@vger.kernel.org >>>Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> >>>--- >>> drivers/vdpa/mlx5/net/mlx5_vnet.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>>diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c >>>index dc88559a8d49..10e9b09932eb 100644 >>>--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c >>>+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c >>>@@ -1820,7 +1820,7 @@ static void mlx5_vdpa_get_config(struct vdpa_device *vdev, unsigned int offset, >>> struct mlx5_vdpa_dev *mvdev = to_mvdev(vdev); >>> struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev); >>>- if (offset + len < sizeof(struct virtio_net_config)) >>>+ if (offset + len <= sizeof(struct virtio_net_config)) >>> memcpy(buf, (u8 *)&ndev->config + offset, len); >>> } >>Actually first I am not sure we need these checks at all. >>vhost_vdpa_config_validate already validates the values, right? > > >I think they're working at different levels. There's no guarantee that >vhost-vdpa is the driver for this vdpa device. >
Maybe we can do these checks in the vdpa_get_config() helper and we can also add a vdpa_set_config() to do the same.
Thanks, Stefano
> >> >>Second, what will happen when we extend the struct and then >>run new userspace on an old kernel? Looks like it will just >>fail right? So what is the plan? > > >In this case, get_config() should match the spec behaviour. That is to >say the size of config space depends on the feature negotiated. > >Thanks > > >> I think we should >>allow a bigger size, and return the copied config size to userspace. >> >> >>>-- >>>2.29.2 >
|  |