On Tue, Nov 7, 2023 at 1:37 PM Albert Esteve <aest...@redhat.com> wrote:
>
> Shared objects lack spoofing protection.
> For VHOST_USER_BACKEND_SHARED_OBJECT_REMOVE messages
> received by the vhost-user interface, any backend was
> allowed to remove entries from the shared table just
> by knowing the UUID. Only the owner of the entry
> shall be allowed to removed their resources
> from the table.
>
> To fix that, add a check for all
> *SHARED_OBJECT_REMOVE messages received.
> A vhost device can only remove TYPE_VHOST_DEV
> entries that are owned by them, otherwise skip
> the removal, and inform the device that the entry
> has not been removed in the answer.
>
> Signed-off-by: Albert Esteve <aest...@redhat.com>
> ---
> hw/virtio/vhost-user.c | 21 +++++++++++++++++++--
> 1 file changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 7b42ae8aae..5fdff0241f 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -1602,10 +1602,26 @@ vhost_user_backend_handle_shared_object_add(struct
> vhost_dev *dev,
> }
>
> static int
> -vhost_user_backend_handle_shared_object_remove(VhostUserShared *object)
> +vhost_user_backend_handle_shared_object_remove(struct vhost_dev *dev,
> + VhostUserShared *object)
> {
> QemuUUID uuid;
>
> + switch (virtio_object_type(&uuid)) {
../hw/virtio/vhost-user.c:1619:13: error: ‘uuid’ may be used
uninitialized [-Werror=maybe-uninitialized]
1619 | switch (virtio_object_type(&uuid)) {
| ^~~~~~~~~~~~~~~~~~~~~~~~~
> + case TYPE_VHOST_DEV:
> + {
> + struct vhost_dev *owner = virtio_lookup_vhost_device(&uuid);
> + if (owner == NULL || dev != owner) {
> + /* Not allowed to remove non-owned entries */
> + return 0;
> + }
> + break;
> + }
> + default:
> + /* Not allowed to remove non-owned entries */
> + return 0;
How do you remove TYPE_DMABUF entries after this patch?
> + }
> +
> memcpy(uuid.data, object->uuid, sizeof(object->uuid));
> return virtio_remove_resource(&uuid);
> }
> @@ -1785,7 +1801,8 @@ static gboolean backend_read(QIOChannel *ioc,
> GIOCondition condition,
> ret = vhost_user_backend_handle_shared_object_add(dev,
> &payload.object);
> break;
> case VHOST_USER_BACKEND_SHARED_OBJECT_REMOVE:
> - ret =
> vhost_user_backend_handle_shared_object_remove(&payload.object);
> + ret = vhost_user_backend_handle_shared_object_remove(dev,
> +
> &payload.object);
> break;
> case VHOST_USER_BACKEND_SHARED_OBJECT_LOOKUP:
> ret = vhost_user_backend_handle_shared_object_lookup(dev->opaque,
> ioc,
> --
> 2.41.0
>
--
Marc-André Lureau
