[Spice-devel] [spice v13 03/29] server: Add a GStreamer 1.0 MJPEG video encoder and use it by default

[Christophe Fergeau]

On Tue, May 03, 2016 at 06:55:36PM +0200, Francois Gouget wrote:
> On Tue, 3 May 2016, Christophe Fergeau wrote:
> [...]
> > > > > +         /* Copy the line */
> > > > > +         uint8_t *src = chunks->chunk[chunk_index].data + chunk_offset;
> > > > > +         memcpy(dst, src, stream_stride);
> > > > 
> > > > Are we guaranteed that we'll have at least 'stream_stride' bytes in the
> > > > chunk?
> > > 
> > > Yes, the is_chunk_padded() check guarantees it. I could rename it to 
> > > is_chunk_stride_aligned() to make it clearer.
> > 
> > Hmm, this guarantees we have at least bitmap->stride bytes, which
> > is (assumed to be?) bigger than stream_stride. Is there an explicit
> > check/reason that bitmap->stride is bigger than stream_stride?
> 
> As far as I can tell, if stream_stride was larger than bitmap->stride it 
> would mean that the video we try to encode is wider than the provided 
> bitmap. I don't think that makes sense or that it can happen.
> 
> 
> > Also, is there anything preventing chunks->chunk[index].len to be 0 in
> > is_chunk_padded()?
> 
> It would mean the chunk contains no data which is already wrong. And it 
> would only be a problem if we still had data to copy which could only 
> happen if the video is higher than the provided bitmap. Again I don't 
> think that makes sense or that it can happen.

Yeah, the only reason for these questions is to know whether a malicious
client could trigger misbehaviour in this part of the code by sending us
some unexpected data. I agree when all is good, this does not make
sense/should not happen, but I'd prefer to be 100% sure it cannot happen
regardless of what we receive from the guest.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20160513/611cff0d/attachment.sig>