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
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel