On 11/1/24 18:35, Peter Maydell wrote: > On Tue, 29 Oct 2024 at 12:11, Alex Bennée <[email protected]> wrote: >> >> From: Robert Beckett <[email protected]> >> >> 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 <[email protected]> >> Signed-off-by: Robert Beckett <[email protected]> # added >> set_scanout_blob >> Signed-off-by: Xenia Ragiadakou <[email protected]> >> Signed-off-by: Huang Rui <[email protected]> >> Signed-off-by: Dmitry Osipenko <[email protected]> >> Message-Id: <[email protected]> >> Signed-off-by: Alex Bennée <[email protected]> > > 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. > > (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.
Will make patch to silence this Coverity warning. AFAICT, there are other ways to cause integer overflows in that code, though I assume it all should be harmless. Thanks! -- Best regards, Dmitry
