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