On Wed, 25 May 2016, Francois Gouget wrote:
> On Tue, 24 May 2016, Francois Gouget wrote:
> [...]
> > Now if the content of SpiceCopy cannot be trusted, the GStreamer encoder
> > is the wrong place to check for issues : there are at least three places
> > where the SpiceCopy bitmap can be used: the MJPEG encoder, the GStreamer
> > encoder and the regular screen update code (which may or may not involve
> > the JPEG encoder) [1].
> >
> > Duplicating checks all over the place can only lead to maintenance
> > issues and incomplete checks. So validating the SpiceCopy structure
> > should be done before it is sent down one or the other path.
>
> To nuance this, in the non-streaming case it seems like the SpiceCopy
> structure is sent to the client without being interpreted. That is the
> image buffer is sent but it does not need to be consistent with its x
> and y dimensions, nor with the src_area struct. If the same is true for
> most other structures, then it would make sense to only perform these
> consistency checks on the server for the streaming code, but before the
> individual video encoders.
I take that back, partially at least. The x and y dimensions on the
SpiceImage must be correct otherwise the server crashes in
dcc_compress_image().
Here's a test for this:
diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c
index 0dafbef..5eddaa1 100644
--- a/server/red-parse-qxl.c
+++ b/server/red-parse-qxl.c
@@ -681,6 +681,7 @@ static int red_get_copy_ptr(RedMemSlotInfo *slots, int group_id,
if (!red->src_bitmap) {
return 1;
}
+ red->src_bitmap->u.bitmap.y += 10;
red_get_rect_ptr(&red->src_area, &qxl->src_area);
red->rop_descriptor = qxl->rop_descriptor;
red->scale_mode = qxl->scale_mode;
So various aspects of SpiceImage should be validated in red_get_image(),
while validating the consistency of the SpiceCopy's src_area field with
the src_bitmap one can be restricted to the streaming case in
red_marshall_stream_data(). However I'm not entirely convinced delaying
that check that much is a good idea.
--
Francois Gouget <fgouget@xxxxxxxxxxxxxxx>
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel