Skip to content

Commit

Permalink
spice-display: reduce race in sending IOSurface
Browse files Browse the repository at this point in the history
There is a kernel bug detailed in
utmapp/UTM#2743 (comment)
which results in a race whenever IOSurfaceGetID/IOSurfaceLookup is used.
As a result, we need a way to "indicate" to the receiver that the
surface id in the pipe is now stale and should not be used. To do this
we send POLLHUP to poll() by closing the write FD when the surface is
about to be deallocated.

Note this does not fix the race completely as there is still a small
chance that the race happens between the close() and the CFRelease() but
the chance of that is small and the whole FD passing surface ID system
is a hack anyways that should be replaced with a proper set of SPICE
APIs one day...
  • Loading branch information
osy committed Feb 22, 2022
1 parent cc24c9b commit 6409730
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 2 deletions.
1 change: 1 addition & 0 deletions include/ui/spice-display.h
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ struct SimpleSpiceDisplay {
#endif
#if defined(CONFIG_IOSURFACE)
IOSurfaceRef iosurface;
int surface_send_fd;
#endif
#if defined(CONFIG_ANGLE)
EGLSurface esurface;
Expand Down
18 changes: 16 additions & 2 deletions ui/spice-display.c
Original file line number Diff line number Diff line change
Expand Up @@ -890,6 +890,13 @@ static void spice_iosurface_destroy(SimpleSpiceDisplay *ssd)
qemu_egl_destroy_surface(ssd->esurface);
ssd->esurface = EGL_NO_SURFACE;
#endif
if (ssd->surface_send_fd > -1) {
// this sends POLLHUP and indicates that any unread data is stale
// and should not be used
close(ssd->surface_send_fd);
ssd->surface_send_fd = -1;
}
// FIXME: still a tiny race with the close() above
CFRelease(ssd->iosurface);
ssd->iosurface = NULL;
}
Expand Down Expand Up @@ -922,10 +929,16 @@ static int spice_iosurface_create_fd(SimpleSpiceDisplay *ssd, int *fourcc)
error_report("spice_iosurface_create_fd: failed to create pipe");
return -1;
}
if (ssd->surface_send_fd > -1) {
close(ssd->surface_send_fd);
}
// we keep the write end of the pipe open for the lifetime of this surface
// when we close it, POLLHUP will be seen by the other side and know that
// the surface ID is stale and should not be used
ssd->surface_send_fd = fds[1];
*fourcc = 'BGRA';
surfaceid = IOSurfaceGetID(ssd->iosurface);
write(fds[1], &surfaceid, sizeof(surfaceid));
close(fds[1]);
write(ssd->surface_send_fd, &surfaceid, sizeof(surfaceid));
return fds[0];
}

Expand Down Expand Up @@ -1381,6 +1394,7 @@ static void qemu_spice_display_init_one(QemuConsole *con)
ssd->have_scanout = false;
#if defined(CONFIG_IOSURFACE)
ssd->iosurface = NULL;
ssd->surface_send_fd = -1;
#endif
#if defined(CONFIG_ANGLE)
ssd->esurface = EGL_NO_SURFACE;
Expand Down

0 comments on commit 6409730

Please sign in to comment.