Primary surface, as additional surfaces, can be used to access host memory from the guest using invalid parameters. The removed warning is not enough to prevent all cases. Also a warning is not enough to stop an escalation to happen. The red_validate_surface do different checks to make sure surface request is valid and not cause possible buffer/integer overflows: - format is valid; - width is not large to cause overflow compared to stride; - stride is not -2^31 (a number which negate is still <0); - stride * height does not overflow. This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1312980. Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx> Acked-by: Christophe Fergeau <cfergeau@xxxxxxxxxx> --- server/red-worker.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/server/red-worker.c b/server/red-worker.c index e754bd2..121a2e5 100644 --- a/server/red-worker.c +++ b/server/red-worker.c @@ -657,8 +657,15 @@ static void dev_create_primary_surface(RedWorker *worker, uint32_t surface_id, spice_debug(NULL); spice_warn_if_fail(surface_id == 0); spice_warn_if_fail(surface.height != 0); - spice_warn_if_fail(((uint64_t)abs(surface.stride) * (uint64_t)surface.height) == - abs(surface.stride) * surface.height); + + /* surface can arrive from guest unchecked so make sure + * guest is not a malicious one and drop invalid requests + */ + if (!red_validate_surface(surface.width, surface.height, + surface.stride, surface.format)) { + spice_warning("wrong primary surface creation request"); + return; + } line_0 = (uint8_t*)memslot_get_virt(&worker->mem_slots, surface.mem, surface.height * abs(surface.stride), -- 2.7.4 _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel