* [Qemu-devel] [PATCH] hw/qxl: vaildate surface->data
@ 2012-10-25 12:27 Alon Levy
2012-11-01 9:33 ` Gerd Hoffmann
0 siblings, 1 reply; 3+ messages in thread
From: Alon Levy @ 2012-10-25 12:27 UTC (permalink / raw)
To: qemu-devel, kraxel
Signed-off-by: Alon Levy <alevy@redhat.com>
---
hw/qxl.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/hw/qxl.c b/hw/qxl.c
index 1b47ed3..620b476 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -453,6 +453,16 @@ static int qxl_track_command(PCIQXLDevice *qxl, struct QXLCommandExt *ext)
cmd->u.surface_create.stride);
return 1;
}
+ if (cmd->type == QXL_SURFACE_CMD_CREATE) {
+ intptr_t surface_offset = (intptr_t)qxl_phys2virt(qxl,
+ cmd->u.surface_create.data,
+ MEMSLOT_GROUP_GUEST);
+ if (!surface_offset) {
+ qxl_set_guest_bug(qxl, "QXL_CMD_SURFACE invalid data: %ld\n",
+ cmd->u.surface_create.data);
+ return 1;
+ }
+ }
qemu_mutex_lock(&qxl->track_lock);
if (cmd->type == QXL_SURFACE_CMD_CREATE) {
qxl->guest_surfaces.cmds[id] = ext->cmd.data;
--
1.7.12.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [Qemu-devel] [PATCH] hw/qxl: vaildate surface->data
2012-10-25 12:27 [Qemu-devel] [PATCH] hw/qxl: vaildate surface->data Alon Levy
@ 2012-11-01 9:33 ` Gerd Hoffmann
2012-11-01 9:43 ` Alon Levy
0 siblings, 1 reply; 3+ messages in thread
From: Gerd Hoffmann @ 2012-11-01 9:33 UTC (permalink / raw)
To: Alon Levy; +Cc: qemu-devel
On 10/25/12 14:27, Alon Levy wrote:
> Signed-off-by: Alon Levy <alevy@redhat.com>
Looks sane at a quick glance.
But: how far we wanna take this? Add checks to qxl for each and every
assert() guests can trigger in spice-server? So we end up
sanity-checking everything twice long-term?
I think instead we'll need a way for spice-server to report back errors
to qxl. So spice-server would just notify qxl and go on (or stop
processing until reset) instead of aborting. qxl in turn will notify
the guest.
[ The alternative would be to basically move server/red_parse_qxl.c
into the qemu codebase. I don't think we want that because that
would make a bunch of data structures which are spice-server internal
today (for good reasons) a libspice-server ABI+API. ]
cheers,
Gerd
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/qxl: vaildate surface->data
2012-11-01 9:33 ` Gerd Hoffmann
@ 2012-11-01 9:43 ` Alon Levy
0 siblings, 0 replies; 3+ messages in thread
From: Alon Levy @ 2012-11-01 9:43 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel
> On 10/25/12 14:27, Alon Levy wrote:
> > Signed-off-by: Alon Levy <alevy@redhat.com>
>
> Looks sane at a quick glance.
>
> But: how far we wanna take this? Add checks to qxl for each and
> every
> assert() guests can trigger in spice-server? So we end up
> sanity-checking everything twice long-term?
>
> I think instead we'll need a way for spice-server to report back
> errors
> to qxl. So spice-server would just notify qxl and go on (or stop
> processing until reset) instead of aborting. qxl in turn will notify
> the guest.
Yes I totally agree but never got around to doing it.
>
> [ The alternative would be to basically move server/red_parse_qxl.c
> into the qemu codebase. I don't think we want that because that
> would make a bunch of data structures which are spice-server
> internal
> today (for good reasons) a libspice-server ABI+API. ]
>
> cheers,
> Gerd
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-11-01 9:44 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-25 12:27 [Qemu-devel] [PATCH] hw/qxl: vaildate surface->data Alon Levy
2012-11-01 9:33 ` Gerd Hoffmann
2012-11-01 9:43 ` Alon Levy
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).