From: Albert Esteve <aesteve@redhat.com>
To: qemu-devel@nongnu.org
Cc: stefanha@gmail.com, Albert Esteve <aesteve@redhat.com>,
"Michael S. Tsirkin" <mst@redhat.com>,
marcandre.lureau@gmail.com, kraxel@redhat.com,
Stefan Hajnoczi <stefanha@redhat.com>
Subject: [PATCH v3 1/3] hw/virtio: check owner for removing objects
Date: Tue, 9 Jan 2024 13:56:12 +0100 [thread overview]
Message-ID: <20240109125614.220293-2-aesteve@redhat.com> (raw)
In-Reply-To: <20240109125614.220293-1-aesteve@redhat.com>
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 <aesteve@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
---
docs/interop/vhost-user.rst | 4 +++-
hw/virtio/vhost-user.c | 21 +++++++++++++++++++--
2 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
index 9f1103f85a..60ec2c9d48 100644
--- a/docs/interop/vhost-user.rst
+++ b/docs/interop/vhost-user.rst
@@ -1839,7 +1839,9 @@ is sent by the front-end.
When the ``VHOST_USER_PROTOCOL_F_SHARED_OBJECT`` protocol
feature has been successfully negotiated, this message can be submitted
by the backend to remove themselves from to the virtio-dmabuf shared
- table API. The shared table will remove the back-end device associated with
+ table API. Only the back-end owning the entry (i.e., the one that first added
+ it) will have permission to remove it. Otherwise, the message is ignored.
+ The shared table will remove the back-end device associated with
the UUID. If ``VHOST_USER_PROTOCOL_F_REPLY_ACK`` is negotiated, and the
back-end sets the ``VHOST_USER_NEED_REPLY`` flag, the front-end must respond
with zero when operation is successfully completed, or non-zero otherwise.
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index f214df804b..1c3f2357be 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -1611,11 +1611,27 @@ 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;
memcpy(uuid.data, object->uuid, sizeof(object->uuid));
+ 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;
+ }
+
return virtio_remove_resource(&uuid);
}
@@ -1794,7 +1810,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.43.0
next prev parent reply other threads:[~2024-01-09 12:57 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-09 12:56 [PATCH v3 0/3] Virtio dmabuf improvements Albert Esteve
2024-01-09 12:56 ` Albert Esteve [this message]
2024-02-05 12:57 ` [PATCH v3 1/3] hw/virtio: check owner for removing objects Alex Bennée
2024-02-15 9:37 ` Albert Esteve
2024-01-09 12:56 ` [PATCH v3 2/3] hw/virtio: cleanup shared resources Albert Esteve
2024-02-05 23:11 ` Alex Bennée
2024-02-15 9:45 ` Albert Esteve
2024-02-15 11:41 ` Alex Bennée
2024-02-19 10:45 ` Albert Esteve
2024-02-19 12:07 ` Albert Esteve
2024-01-09 12:56 ` [PATCH v3 3/3] hw/virtio: rename virtio dmabuf API Albert Esteve
2024-01-09 21:14 ` Philippe Mathieu-Daudé
2024-02-05 9:58 ` [PATCH v3 0/3] Virtio dmabuf improvements Albert Esteve
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240109125614.220293-2-aesteve@redhat.com \
--to=aesteve@redhat.com \
--cc=kraxel@redhat.com \
--cc=marcandre.lureau@gmail.com \
--cc=mst@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@gmail.com \
--cc=stefanha@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).