* [PATCH 0/2] virtio-gpu: coverity fixes @ 2024-11-04 16:53 Alex Bennée 2024-11-04 16:53 ` [PATCH 1/2] hw/display: factor out the scanout blob to fb conversion Alex Bennée 2024-11-04 16:53 ` [PATCH 2/2] hw/display: check frame buffer can hold blob Alex Bennée 0 siblings, 2 replies; 6+ messages in thread From: Alex Bennée @ 2024-11-04 16:53 UTC (permalink / raw) To: qemu-devel; +Cc: Michael S. Tsirkin, Alex Bennée Hi Dimitry, I'd already started this before I saw your email so I thought I might as well push what I had. Feel free to review/use as a base/ignore as you wish ;-) Alex. Alex Bennée (2): hw/display: factor out the scanout blob to fb conversion hw/display: check frame buffer can hold blob include/hw/virtio/virtio-gpu.h | 15 ++++++++ hw/display/virtio-gpu-virgl.c | 21 +---------- hw/display/virtio-gpu.c | 65 ++++++++++++++++++++++------------ 3 files changed, 58 insertions(+), 43 deletions(-) -- 2.39.5 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] hw/display: factor out the scanout blob to fb conversion 2024-11-04 16:53 [PATCH 0/2] virtio-gpu: coverity fixes Alex Bennée @ 2024-11-04 16:53 ` Alex Bennée 2024-11-06 0:33 ` Dmitry Osipenko 2024-11-04 16:53 ` [PATCH 2/2] hw/display: check frame buffer can hold blob Alex Bennée 1 sibling, 1 reply; 6+ messages in thread From: Alex Bennée @ 2024-11-04 16:53 UTC (permalink / raw) To: qemu-devel; +Cc: Michael S. Tsirkin, Alex Bennée, Dmitry Osipenko There are two identical sequences of a code doing the same thing that raise warnings with Coverity. Before fixing those issues lets factor out the common code into a helper function we can share. Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Cc: Dmitry Osipenko <dmitry.osipenko@collabora.com> --- include/hw/virtio/virtio-gpu.h | 15 +++++++++ hw/display/virtio-gpu-virgl.c | 21 +----------- hw/display/virtio-gpu.c | 60 +++++++++++++++++++++------------- 3 files changed, 53 insertions(+), 43 deletions(-) diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h index 553799b8cc..90e4abe788 100644 --- a/include/hw/virtio/virtio-gpu.h +++ b/include/hw/virtio/virtio-gpu.h @@ -333,6 +333,21 @@ void virtio_gpu_update_cursor_data(VirtIOGPU *g, struct virtio_gpu_scanout *s, uint32_t resource_id); +/** + * virtio_gpu_scanout_blob_to_fb() - fill out fb based on scanout data + * fb: the frame-buffer descriptor to fill out + * ss: the scanout blob data + * blob_size: the maximum size the blob can accommodate + * + * This will check we have enough space for the frame taking into + * account that stride for all but the last line. + * + * Returns true on success, otherwise logs guest error and returns false + */ +bool virtio_gpu_scanout_blob_to_fb(struct virtio_gpu_framebuffer *fb, + struct virtio_gpu_set_scanout_blob *ss, + uint64_t blob_size); + /* virtio-gpu-udmabuf.c */ bool virtio_gpu_have_udmabuf(void); void virtio_gpu_init_udmabuf(struct virtio_gpu_simple_resource *res); diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c index eedae7357f..35599cddab 100644 --- a/hw/display/virtio-gpu-virgl.c +++ b/hw/display/virtio-gpu-virgl.c @@ -852,26 +852,7 @@ static void virgl_cmd_set_scanout_blob(VirtIOGPU *g, return; } - fb.format = virtio_gpu_get_pixman_format(ss.format); - if (!fb.format) { - qemu_log_mask(LOG_GUEST_ERROR, "%s: pixel format not supported %d\n", - __func__, ss.format); - cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER; - return; - } - - fb.bytes_pp = DIV_ROUND_UP(PIXMAN_FORMAT_BPP(fb.format), 8); - fb.width = ss.width; - fb.height = ss.height; - fb.stride = ss.strides[0]; - fb.offset = ss.offsets[0] + ss.r.x * fb.bytes_pp + ss.r.y * fb.stride; - - fbend = fb.offset; - fbend += fb.stride * (ss.r.height - 1); - fbend += fb.bytes_pp * ss.r.width; - if (fbend > res->base.blob_size) { - qemu_log_mask(LOG_GUEST_ERROR, "%s: fb end out of range\n", - __func__); + if (!virtio_gpu_scanout_blob_to_fb(&fb, &ss, res->blob_size)) { cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER; return; } diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c index c0570ef856..e7ca8fd1cf 100644 --- a/hw/display/virtio-gpu.c +++ b/hw/display/virtio-gpu.c @@ -721,13 +721,48 @@ static void virtio_gpu_set_scanout(VirtIOGPU *g, &fb, res, &ss.r, &cmd->error); } +bool virtio_gpu_scanout_blob_to_fb(struct virtio_gpu_framebuffer *fb, + struct virtio_gpu_set_scanout_blob *ss, + uint64_t blob_size) +{ + uint64_t fbend; + + fb->format = virtio_gpu_get_pixman_format(ss->format); + if (!fb->format) { + qemu_log_mask(LOG_GUEST_ERROR, + "%s: host couldn't handle guest format %d\n", + __func__, ss->format); + return false; + } + + fb->bytes_pp = DIV_ROUND_UP(PIXMAN_FORMAT_BPP(fb->format), 8); + fb->width = ss->width; + fb->height = ss->height; + fb->stride = ss->strides[0]; + fb->offset = ss->offsets[0] + ss->r.x * fb->bytes_pp + ss->r.y * fb->stride; + + fbend = fb->offset; + fbend += fb->stride * (ss->r.height - 1); + fbend += fb->bytes_pp * ss->r.width; + + if (fbend > blob_size) { + qemu_log_mask(LOG_GUEST_ERROR, + "%s: fb end out of range\n", + __func__); + return false; + } + + return true; +} + + + static void virtio_gpu_set_scanout_blob(VirtIOGPU *g, struct virtio_gpu_ctrl_command *cmd) { struct virtio_gpu_simple_resource *res; struct virtio_gpu_framebuffer fb = { 0 }; struct virtio_gpu_set_scanout_blob ss; - uint64_t fbend; VIRTIO_GPU_FILL_CMD(ss); virtio_gpu_scanout_blob_bswap(&ss); @@ -753,28 +788,7 @@ static void virtio_gpu_set_scanout_blob(VirtIOGPU *g, return; } - fb.format = virtio_gpu_get_pixman_format(ss.format); - if (!fb.format) { - qemu_log_mask(LOG_GUEST_ERROR, - "%s: host couldn't handle guest format %d\n", - __func__, ss.format); - cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER; - return; - } - - fb.bytes_pp = DIV_ROUND_UP(PIXMAN_FORMAT_BPP(fb.format), 8); - fb.width = ss.width; - fb.height = ss.height; - fb.stride = ss.strides[0]; - fb.offset = ss.offsets[0] + ss.r.x * fb.bytes_pp + ss.r.y * fb.stride; - - fbend = fb.offset; - fbend += fb.stride * (ss.r.height - 1); - fbend += fb.bytes_pp * ss.r.width; - if (fbend > res->blob_size) { - qemu_log_mask(LOG_GUEST_ERROR, - "%s: fb end out of range\n", - __func__); + if (!virtio_gpu_scanout_blob_to_fb(&fb, &ss, res->blob_size)) { cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER; return; } -- 2.39.5 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] hw/display: factor out the scanout blob to fb conversion 2024-11-04 16:53 ` [PATCH 1/2] hw/display: factor out the scanout blob to fb conversion Alex Bennée @ 2024-11-06 0:33 ` Dmitry Osipenko 2024-11-06 17:36 ` Alex Bennée 0 siblings, 1 reply; 6+ messages in thread From: Dmitry Osipenko @ 2024-11-06 0:33 UTC (permalink / raw) To: Alex Bennée, qemu-devel; +Cc: Michael S. Tsirkin On 11/4/24 19:53, Alex Bennée wrote: > There are two identical sequences of a code doing the same thing that > raise warnings with Coverity. Before fixing those issues lets factor > out the common code into a helper function we can share. > > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > Cc: Dmitry Osipenko <dmitry.osipenko@collabora.com> > --- > include/hw/virtio/virtio-gpu.h | 15 +++++++++ > hw/display/virtio-gpu-virgl.c | 21 +----------- > hw/display/virtio-gpu.c | 60 +++++++++++++++++++++------------- > 3 files changed, 53 insertions(+), 43 deletions(-) > > diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h > index 553799b8cc..90e4abe788 100644 > --- a/include/hw/virtio/virtio-gpu.h > +++ b/include/hw/virtio/virtio-gpu.h > @@ -333,6 +333,21 @@ void virtio_gpu_update_cursor_data(VirtIOGPU *g, > struct virtio_gpu_scanout *s, > uint32_t resource_id); > > +/** > + * virtio_gpu_scanout_blob_to_fb() - fill out fb based on scanout data > + * fb: the frame-buffer descriptor to fill out > + * ss: the scanout blob data > + * blob_size: the maximum size the blob can accommodate Nit: 'maximum size the blob can accommodate' makes it sound to me like data will be copied into the blob. What about 'size of scanout blob data'. > + * > + * This will check we have enough space for the frame taking into > + * account that stride for all but the last line. > + * > + * Returns true on success, otherwise logs guest error and returns false > + */ > +bool virtio_gpu_scanout_blob_to_fb(struct virtio_gpu_framebuffer *fb, > + struct virtio_gpu_set_scanout_blob *ss, > + uint64_t blob_size); > + > /* virtio-gpu-udmabuf.c */ > bool virtio_gpu_have_udmabuf(void); > void virtio_gpu_init_udmabuf(struct virtio_gpu_simple_resource *res); > diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c > index eedae7357f..35599cddab 100644 > --- a/hw/display/virtio-gpu-virgl.c > +++ b/hw/display/virtio-gpu-virgl.c > @@ -852,26 +852,7 @@ static void virgl_cmd_set_scanout_blob(VirtIOGPU *g, > return; > } > > - fb.format = virtio_gpu_get_pixman_format(ss.format); > - if (!fb.format) { > - qemu_log_mask(LOG_GUEST_ERROR, "%s: pixel format not supported %d\n", > - __func__, ss.format); > - cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER; > - return; > - } > - > - fb.bytes_pp = DIV_ROUND_UP(PIXMAN_FORMAT_BPP(fb.format), 8); > - fb.width = ss.width; > - fb.height = ss.height; > - fb.stride = ss.strides[0]; > - fb.offset = ss.offsets[0] + ss.r.x * fb.bytes_pp + ss.r.y * fb.stride; > - > - fbend = fb.offset; > - fbend += fb.stride * (ss.r.height - 1); > - fbend += fb.bytes_pp * ss.r.width; > - if (fbend > res->base.blob_size) { > - qemu_log_mask(LOG_GUEST_ERROR, "%s: fb end out of range\n", > - __func__); > + if (!virtio_gpu_scanout_blob_to_fb(&fb, &ss, res->blob_size)) { This fails to compile, needs s/res->blob_size/res->base.blob_size/ ../hw/display/virtio-gpu-virgl.c:855:53: error: 'struct virtio_gpu_virgl_resource' has no member named 'blob_size' 855 | if (!virtio_gpu_scanout_blob_to_fb(&fb, &ss, res->blob_size)) { | ^~ ../hw/display/virtio-gpu-virgl.c:808:14: error: unused variable 'fbend' [-Werror=unused-variable] 808 | uint64_t fbend; | ^~~~~ cc1: all warnings being treated as errors Please correct in v2. > cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER; > return; > } > diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c > index c0570ef856..e7ca8fd1cf 100644 > --- a/hw/display/virtio-gpu.c > +++ b/hw/display/virtio-gpu.c > @@ -721,13 +721,48 @@ static void virtio_gpu_set_scanout(VirtIOGPU *g, > &fb, res, &ss.r, &cmd->error); > } > > +bool virtio_gpu_scanout_blob_to_fb(struct virtio_gpu_framebuffer *fb, > + struct virtio_gpu_set_scanout_blob *ss, > + uint64_t blob_size) > +{ > + uint64_t fbend; > + > + fb->format = virtio_gpu_get_pixman_format(ss->format); > + if (!fb->format) { > + qemu_log_mask(LOG_GUEST_ERROR, > + "%s: host couldn't handle guest format %d\n", > + __func__, ss->format); > + return false; > + } > + > + fb->bytes_pp = DIV_ROUND_UP(PIXMAN_FORMAT_BPP(fb->format), 8); > + fb->width = ss->width; > + fb->height = ss->height; > + fb->stride = ss->strides[0]; > + fb->offset = ss->offsets[0] + ss->r.x * fb->bytes_pp + ss->r.y * fb->stride; > + > + fbend = fb->offset; > + fbend += fb->stride * (ss->r.height - 1); > + fbend += fb->bytes_pp * ss->r.width; > + > + if (fbend > blob_size) { > + qemu_log_mask(LOG_GUEST_ERROR, > + "%s: fb end out of range\n", > + __func__); > + return false; > + } > + > + return true; > +} > + > + > + Nit: extra newlines -- Best regards, Dmitry ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] hw/display: factor out the scanout blob to fb conversion 2024-11-06 0:33 ` Dmitry Osipenko @ 2024-11-06 17:36 ` Alex Bennée 0 siblings, 0 replies; 6+ messages in thread From: Alex Bennée @ 2024-11-06 17:36 UTC (permalink / raw) To: Dmitry Osipenko; +Cc: qemu-devel, Michael S. Tsirkin Dmitry Osipenko <dmitry.osipenko@collabora.com> writes: > On 11/4/24 19:53, Alex Bennée wrote: >> There are two identical sequences of a code doing the same thing that >> raise warnings with Coverity. Before fixing those issues lets factor >> out the common code into a helper function we can share. >> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >> Cc: Dmitry Osipenko <dmitry.osipenko@collabora.com> >> --- >> include/hw/virtio/virtio-gpu.h | 15 +++++++++ >> hw/display/virtio-gpu-virgl.c | 21 +----------- >> hw/display/virtio-gpu.c | 60 +++++++++++++++++++++------------- >> 3 files changed, 53 insertions(+), 43 deletions(-) >> >> diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h >> index 553799b8cc..90e4abe788 100644 >> --- a/include/hw/virtio/virtio-gpu.h >> +++ b/include/hw/virtio/virtio-gpu.h >> @@ -333,6 +333,21 @@ void virtio_gpu_update_cursor_data(VirtIOGPU *g, >> struct virtio_gpu_scanout *s, >> uint32_t resource_id); >> >> +/** >> + * virtio_gpu_scanout_blob_to_fb() - fill out fb based on scanout data >> + * fb: the frame-buffer descriptor to fill out >> + * ss: the scanout blob data >> + * blob_size: the maximum size the blob can accommodate > > Nit: 'maximum size the blob can accommodate' makes it sound to me like > data will be copied into the blob. What about 'size of scanout blob data'. > >> + * >> + * This will check we have enough space for the frame taking into >> + * account that stride for all but the last line. >> + * >> + * Returns true on success, otherwise logs guest error and returns false >> + */ >> +bool virtio_gpu_scanout_blob_to_fb(struct virtio_gpu_framebuffer *fb, >> + struct virtio_gpu_set_scanout_blob *ss, >> + uint64_t blob_size); >> + >> /* virtio-gpu-udmabuf.c */ >> bool virtio_gpu_have_udmabuf(void); >> void virtio_gpu_init_udmabuf(struct virtio_gpu_simple_resource *res); >> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c >> index eedae7357f..35599cddab 100644 >> --- a/hw/display/virtio-gpu-virgl.c >> +++ b/hw/display/virtio-gpu-virgl.c >> @@ -852,26 +852,7 @@ static void virgl_cmd_set_scanout_blob(VirtIOGPU *g, >> return; >> } >> >> - fb.format = virtio_gpu_get_pixman_format(ss.format); >> - if (!fb.format) { >> - qemu_log_mask(LOG_GUEST_ERROR, "%s: pixel format not supported %d\n", >> - __func__, ss.format); >> - cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER; >> - return; >> - } >> - >> - fb.bytes_pp = DIV_ROUND_UP(PIXMAN_FORMAT_BPP(fb.format), 8); >> - fb.width = ss.width; >> - fb.height = ss.height; >> - fb.stride = ss.strides[0]; >> - fb.offset = ss.offsets[0] + ss.r.x * fb.bytes_pp + ss.r.y * fb.stride; >> - >> - fbend = fb.offset; >> - fbend += fb.stride * (ss.r.height - 1); >> - fbend += fb.bytes_pp * ss.r.width; >> - if (fbend > res->base.blob_size) { >> - qemu_log_mask(LOG_GUEST_ERROR, "%s: fb end out of range\n", >> - __func__); >> + if (!virtio_gpu_scanout_blob_to_fb(&fb, &ss, res->blob_size)) { > > This fails to compile, needs s/res->blob_size/res->base.blob_size/ > > ../hw/display/virtio-gpu-virgl.c:855:53: error: 'struct > virtio_gpu_virgl_resource' has no member named 'blob_size' > 855 | if (!virtio_gpu_scanout_blob_to_fb(&fb, &ss, res->blob_size)) { > | ^~ > ../hw/display/virtio-gpu-virgl.c:808:14: error: unused variable 'fbend' > [-Werror=unused-variable] > 808 | uint64_t fbend; > | ^~~~~ > cc1: all warnings being treated as errors > > Please correct in v2. Doh - I failed to compile that in my extra.libs config. Will fix. > >> cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER; >> return; >> } >> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c >> index c0570ef856..e7ca8fd1cf 100644 >> --- a/hw/display/virtio-gpu.c >> +++ b/hw/display/virtio-gpu.c >> @@ -721,13 +721,48 @@ static void virtio_gpu_set_scanout(VirtIOGPU *g, >> &fb, res, &ss.r, &cmd->error); >> } >> >> +bool virtio_gpu_scanout_blob_to_fb(struct virtio_gpu_framebuffer *fb, >> + struct virtio_gpu_set_scanout_blob *ss, >> + uint64_t blob_size) >> +{ >> + uint64_t fbend; >> + >> + fb->format = virtio_gpu_get_pixman_format(ss->format); >> + if (!fb->format) { >> + qemu_log_mask(LOG_GUEST_ERROR, >> + "%s: host couldn't handle guest format %d\n", >> + __func__, ss->format); >> + return false; >> + } >> + >> + fb->bytes_pp = DIV_ROUND_UP(PIXMAN_FORMAT_BPP(fb->format), 8); >> + fb->width = ss->width; >> + fb->height = ss->height; >> + fb->stride = ss->strides[0]; >> + fb->offset = ss->offsets[0] + ss->r.x * fb->bytes_pp + ss->r.y * fb->stride; >> + >> + fbend = fb->offset; >> + fbend += fb->stride * (ss->r.height - 1); >> + fbend += fb->bytes_pp * ss->r.width; >> + >> + if (fbend > blob_size) { >> + qemu_log_mask(LOG_GUEST_ERROR, >> + "%s: fb end out of range\n", >> + __func__); >> + return false; >> + } >> + >> + return true; >> +} >> + >> + >> + > > Nit: extra newlines -- Alex Bennée Virtualisation Tech Lead @ Linaro ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] hw/display: check frame buffer can hold blob 2024-11-04 16:53 [PATCH 0/2] virtio-gpu: coverity fixes Alex Bennée 2024-11-04 16:53 ` [PATCH 1/2] hw/display: factor out the scanout blob to fb conversion Alex Bennée @ 2024-11-04 16:53 ` Alex Bennée 2024-11-06 0:56 ` Dmitry Osipenko 1 sibling, 1 reply; 6+ messages in thread From: Alex Bennée @ 2024-11-04 16:53 UTC (permalink / raw) To: qemu-devel; +Cc: Michael S. Tsirkin, Alex Bennée, Dmitry Osipenko Coverity reports (CID 1564769, 1564770) that we potentially overflow by doing some 32x32 multiplies for something that ends up in a 64 bit value. Fix this by casting the first input to uint64_t to ensure a 64 bit multiply is used. While we are at it note why we split the calculation into stride and bytes_pp parts. Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Cc: Dmitry Osipenko <dmitry.osipenko@collabora.com> --- hw/display/virtio-gpu.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c index e7ca8fd1cf..572e4d92c6 100644 --- a/hw/display/virtio-gpu.c +++ b/hw/display/virtio-gpu.c @@ -741,9 +741,14 @@ bool virtio_gpu_scanout_blob_to_fb(struct virtio_gpu_framebuffer *fb, fb->stride = ss->strides[0]; fb->offset = ss->offsets[0] + ss->r.x * fb->bytes_pp + ss->r.y * fb->stride; + /* + * We calculate fb->stride for every line but the last which we + * calculate purely by its width. The stride will often be larger + * than width to meet alignment requirements. + */ fbend = fb->offset; - fbend += fb->stride * (ss->r.height - 1); - fbend += fb->bytes_pp * ss->r.width; + fbend += (uint64_t) fb->stride * (ss->r.height - 1); + fbend += (uint64_t) fb->bytes_pp * ss->r.width; if (fbend > blob_size) { qemu_log_mask(LOG_GUEST_ERROR, -- 2.39.5 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] hw/display: check frame buffer can hold blob 2024-11-04 16:53 ` [PATCH 2/2] hw/display: check frame buffer can hold blob Alex Bennée @ 2024-11-06 0:56 ` Dmitry Osipenko 0 siblings, 0 replies; 6+ messages in thread From: Dmitry Osipenko @ 2024-11-06 0:56 UTC (permalink / raw) To: Alex Bennée, qemu-devel; +Cc: Michael S. Tsirkin On 11/4/24 19:53, Alex Bennée wrote: > Coverity reports (CID 1564769, 1564770) that we potentially overflow > by doing some 32x32 multiplies for something that ends up in a 64 bit > value. Fix this by casting the first input to uint64_t to ensure a 64 > bit multiply is used. > > While we are at it note why we split the calculation into stride and > bytes_pp parts. > > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > Cc: Dmitry Osipenko <dmitry.osipenko@collabora.com> > --- > hw/display/virtio-gpu.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c > index e7ca8fd1cf..572e4d92c6 100644 > --- a/hw/display/virtio-gpu.c > +++ b/hw/display/virtio-gpu.c > @@ -741,9 +741,14 @@ bool virtio_gpu_scanout_blob_to_fb(struct virtio_gpu_framebuffer *fb, > fb->stride = ss->strides[0]; > fb->offset = ss->offsets[0] + ss->r.x * fb->bytes_pp + ss->r.y * fb->stride; > > + /* > + * We calculate fb->stride for every line but the last which we > + * calculate purely by its width. The stride will often be larger > + * than width to meet alignment requirements. > + */ > fbend = fb->offset; > - fbend += fb->stride * (ss->r.height - 1); > - fbend += fb->bytes_pp * ss->r.width; > + fbend += (uint64_t) fb->stride * (ss->r.height - 1); ss->r.height=0 will result into overflow. I don't see why the last line needs to be treated differently, that's wrong. The last line shall have same stride as other lines, otherwise it may result into OOB reading of the last line depending on the reader implementation. Let's fix it too, all lines should have same stride. -- Best regards, Dmitry ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-11-06 17:37 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-11-04 16:53 [PATCH 0/2] virtio-gpu: coverity fixes Alex Bennée 2024-11-04 16:53 ` [PATCH 1/2] hw/display: factor out the scanout blob to fb conversion Alex Bennée 2024-11-06 0:33 ` Dmitry Osipenko 2024-11-06 17:36 ` Alex Bennée 2024-11-04 16:53 ` [PATCH 2/2] hw/display: check frame buffer can hold blob Alex Bennée 2024-11-06 0:56 ` Dmitry Osipenko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).