Peter Maydell <peter.mayd...@linaro.org> writes:
> On Tue, 29 Oct 2024 at 12:11, Alex Bennée <alex.ben...@linaro.org> wrote:
>>
>> From: Robert Beckett <bob.beck...@collabora.com>
>>
>> Support BLOB resources creation, mapping, unmapping and set-scanout by
>> calling the new stable virglrenderer 0.10 interface. Only enabled when
>> available and via the blob config. E.g. -device virtio-vga-gl,blob=true
>>
>> Signed-off-by: Antonio Caggiano <antonio.caggi...@collabora.com>
>> Signed-off-by: Robert Beckett <bob.beck...@collabora.com> # added
>> set_scanout_blob
>> Signed-off-by: Xenia Ragiadakou <xenia.ragiada...@amd.com>
>> Signed-off-by: Huang Rui <ray.hu...@amd.com>
>> Signed-off-by: Dmitry Osipenko <dmitry.osipe...@collabora.com>
>> Message-Id: <20241024210311.118220-12-dmitry.osipe...@collabora.com>
>> Signed-off-by: Alex Bennée <alex.ben...@linaro.org>
>
> Hi; Coverity points out some possible issues with this commit:
>
>
>> + 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;
>
> Here 'fbend' is a uint64_t, but fb.stride, fb.bytes_pp,
> ss.r.height and ss.r.width are all uint32_t. So these
> multiplications will be done as 32x32->32 before being
> added to fbend, potentially overflowing. This probably
> isn't what was intended.
Also what is the subtlety behind using both stride and bytes_pp in the
calculation. My naive thought would be:
fb.bytes_pp * ss.r.width == fb.stride
Can anyone enlighten me?
> (This is Coverity CID 1564769, 1564770.)
>
> The calculation of fb.offset also might overflow, but
> Coverity doesn't spot that because fb.offset is uint32_t
> and so the whole calculation is 32-bit all the way through
> without a late-32-to-64 extension.
I'll roll a patch once I understand this (and fixup the other case as
well).
>
>> + if (fbend > res->base.blob_size) {
>> + qemu_log_mask(LOG_GUEST_ERROR, "%s: fb end out of range\n",
>> + __func__);
>> + cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
>> + return;
>> + }
>
> thanks
> -- PMM
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
