Re: [RFC PATCH-for-7.2 3/5] hw/display/qxl: Pass requested buffer size to qxl_phys2virt()

[Philippe Mathieu-Daudé]

On 28/11/22 16:08, Gerd Hoffmann wrote:
> > @@ -228,7 +230,7 @@ static void qxl_unpack_chunks(void *dest, size_t size, 
> > PCIQXLDevice *qxl,
> >           if (offset == size) {
> >               return;
> >           }
> > -        chunk = qxl_phys2virt(qxl, chunk->next_chunk, group_id);
> > +        chunk = qxl_phys2virt(qxl, chunk->next_chunk, group_id, bytes);
> >           if (!chunk) {
> >               return;
> >           }
>
> Naa, its not that simple.  You get a QXLDataChunk passed in which
> typically is verified *excluding* dynamically-sized chunk->data.

OK so IIUC 1/ this line should be:

  chunk = qxl_phys2virt(qxl, chunk->next_chunk, group_id,
                        sizeof(QXLDataChunk));

but 2/ we should check chunk->data[chunk->data_size] is valid (within
the MR) before calling the memcpy(), right?

> Also at least one code path (processing SPICE_CURSOR_TYPE_MONO in
> qxl_cursor) goes access chunk.data[] without calling
> qxl_unpack_chunks(), that needs additional verification too (or
> switch it to call qxl_unpack_chunks, or just drop it because nobody
> uses mono chrome cursors anyway).

OK I'll look at that.

Thanks,

Phil.