| Message ID | 20181008185013.19371-1-mreitz@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | sdl2: Support all virtio-gpu formats | expand |
On 08.10.18 20:50, Max Reitz wrote: > There are some 2D resource formats that can be used through virtio-gpu, > but which are not supported by SDL2 when used for a scanout; these are > all alpha-channel formats and also XBGR (RGBX in non-BE pixman). Oops, it's the other way round. The virtio-gpu format is RGBX, and the non-BE pixman (and SDL) constant is XBGR. Max > Add these formats in the switch converting pixman to SDL format > constants so a guest cannot crash the VM by triggering the > g_assert_not_reached() with an unsupported format. > > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > ui/sdl2-2d.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/ui/sdl2-2d.c b/ui/sdl2-2d.c > index 85484407be..c9df72636a 100644 > --- a/ui/sdl2-2d.c > +++ b/ui/sdl2-2d.c > @@ -101,15 +101,24 @@ void sdl2_2d_switch(DisplayChangeListener *dcl, > case PIXMAN_r5g6b5: > format = SDL_PIXELFORMAT_RGB565; > break; > + case PIXMAN_a8r8g8b8: > case PIXMAN_x8r8g8b8: > format = SDL_PIXELFORMAT_ARGB8888; > break; > + case PIXMAN_a8b8g8r8: > + case PIXMAN_x8b8g8r8: > + format = SDL_PIXELFORMAT_ABGR8888; > + break; > + case PIXMAN_r8g8b8a8: > case PIXMAN_r8g8b8x8: > format = SDL_PIXELFORMAT_RGBA8888; > break; > case PIXMAN_b8g8r8x8: > format = SDL_PIXELFORMAT_BGRX8888; > break; > + case PIXMAN_b8g8r8a8: > + format = SDL_PIXELFORMAT_BGRA8888; > + break; > default: > g_assert_not_reached(); > } >
On Mon, Oct 08, 2018 at 08:50:13PM +0200, Max Reitz wrote: > There are some 2D resource formats that can be used through virtio-gpu, Ahem, not really. XRGB is the only one which works in practice, and virtio-gpu kms driver will stop advertising anything else soon (patches should land upstream with the next merge window). > Add these formats in the switch converting pixman to SDL format > constants so a guest cannot crash the VM by triggering the > g_assert_not_reached() with an unsupported format. Do you have a reproducer for that? There is sdl2_2d_check_format() which reports the supported formats. If we hit sdl2_2d_switch() with a format not whitelisted by sdl2_2d_check_format() we have a bug somewhere in qemu ... cheers, Gerd
On 10.10.18 12:10, Gerd Hoffmann wrote: > On Mon, Oct 08, 2018 at 08:50:13PM +0200, Max Reitz wrote: >> There are some 2D resource formats that can be used through virtio-gpu, > > Ahem, not really. XRGB is the only one which works in practice, and > virtio-gpu kms driver will stop advertising anything else soon (patches > should land upstream with the next merge window). OK, if virtio-gpu didn't support anything else, that'd be a fix, too. But it sounds like you're talking about the Linux driver, I'm not. This is not about Linux applications being able to abuse the Linux driver to crash the VM, this is about malicious drivers (not necessarily Linux drivers). >> Add these formats in the switch converting pixman to SDL format >> constants so a guest cannot crash the VM by triggering the >> g_assert_not_reached() with an unsupported format. > > Do you have a reproducer for that? I have attached two RISC-V kernels, one (kernel-rgbx) setting VIRTIO_GPU_FORMAT_R8G8B8X8_UNORM, the other (kernel-bgra) setting VIRTIO_GPU_FORMAT_B8G8R8A8_UNORM. Both crash qemu: $ $QEMU/build/riscv64-softmmu/qemu-system-riscv64 -kernel kernel-rgbx \ -serial stdio -M virt -device virtio-gpu-device [platform-virt] Virt platform detected [virtio-gpu] Found device @0x10008000 [virtio-gpu] Scanout 0: 0x0:1024x768 ** ERROR:$QEMU/ui/sdl2-2d.c:114:sdl2_2d_switch: code should not be reached [1] 7151 abort (core dumped) So this is not about a misbehaving Linux driver but about an own driver. Of course, if you can insert kernel code, there's noone stopping you from hitting that assertion with Linux, too. > There is sdl2_2d_check_format() which reports the supported formats. > If we hit sdl2_2d_switch() with a format not whitelisted by > sdl2_2d_check_format() we have a bug somewhere in qemu ... I suppose the other solution would be for virtio_gpu_set_scanout() to check whether the resource's format can actually be used for that display. Or in virtio_gpu_resource_create_2d(), I don't know whether it's possible to use resources in other formats at all. Max
On 10.10.18 19:35, Max Reitz wrote: > On 10.10.18 12:10, Gerd Hoffmann wrote: >> On Mon, Oct 08, 2018 at 08:50:13PM +0200, Max Reitz wrote: >>> There are some 2D resource formats that can be used through virtio-gpu, >> >> Ahem, not really. XRGB is the only one which works in practice, and >> virtio-gpu kms driver will stop advertising anything else soon (patches >> should land upstream with the next merge window). > > OK, if virtio-gpu didn't support anything else, that'd be a fix, too. > But it sounds like you're talking about the Linux driver, I'm not. > > This is not about Linux applications being able to abuse the Linux > driver to crash the VM, this is about malicious drivers (not necessarily > Linux drivers). > >>> Add these formats in the switch converting pixman to SDL format >>> constants so a guest cannot crash the VM by triggering the >>> g_assert_not_reached() with an unsupported format. >> >> Do you have a reproducer for that? > > I have attached two RISC-V kernels, one (kernel-rgbx) setting > VIRTIO_GPU_FORMAT_R8G8B8X8_UNORM, the other (kernel-bgra) setting > VIRTIO_GPU_FORMAT_B8G8R8A8_UNORM. Both crash qemu: Of course I hadn't. > $ $QEMU/build/riscv64-softmmu/qemu-system-riscv64 -kernel kernel-rgbx \ > -serial stdio -M virt -device virtio-gpu-device > [platform-virt] Virt platform detected > [virtio-gpu] Found device @0x10008000 > [virtio-gpu] Scanout 0: 0x0:1024x768 > ** > ERROR:$QEMU/ui/sdl2-2d.c:114:sdl2_2d_switch: code should not be reached > [1] 7151 abort (core dumped) > > So this is not about a misbehaving Linux driver but about an own driver. > Of course, if you can insert kernel code, there's noone stopping you > from hitting that assertion with Linux, too. > >> There is sdl2_2d_check_format() which reports the supported formats. >> If we hit sdl2_2d_switch() with a format not whitelisted by >> sdl2_2d_check_format() we have a bug somewhere in qemu ... > > I suppose the other solution would be for virtio_gpu_set_scanout() to > check whether the resource's format can actually be used for that > display. Or in virtio_gpu_resource_create_2d(), I don't know whether > it's possible to use resources in other formats at all. > > Max >
On Wed, Oct 10, 2018 at 07:35:06PM +0200, Max Reitz wrote: > On 10.10.18 12:10, Gerd Hoffmann wrote: > > On Mon, Oct 08, 2018 at 08:50:13PM +0200, Max Reitz wrote: > >> There are some 2D resource formats that can be used through virtio-gpu, > > > > Ahem, not really. XRGB is the only one which works in practice, and > > virtio-gpu kms driver will stop advertising anything else soon (patches > > should land upstream with the next merge window). > > OK, if virtio-gpu didn't support anything else, that'd be a fix, too. > But it sounds like you're talking about the Linux driver, I'm not. > > This is not about Linux applications being able to abuse the Linux > driver to crash the VM, this is about malicious drivers (not necessarily > Linux drivers). > > >> Add these formats in the switch converting pixman to SDL format > >> constants so a guest cannot crash the VM by triggering the > >> g_assert_not_reached() with an unsupported format. > > > > Do you have a reproducer for that? > > I have attached two RISC-V kernels, one (kernel-rgbx) setting > VIRTIO_GPU_FORMAT_R8G8B8X8_UNORM, the other (kernel-bgra) setting > VIRTIO_GPU_FORMAT_B8G8R8A8_UNORM. Both crash qemu: > > $ $QEMU/build/riscv64-softmmu/qemu-system-riscv64 -kernel kernel-rgbx \ > -serial stdio -M virt -device virtio-gpu-device > [platform-virt] Virt platform detected > [virtio-gpu] Found device @0x10008000 > [virtio-gpu] Scanout 0: 0x0:1024x768 > ** > ERROR:$QEMU/ui/sdl2-2d.c:114:sdl2_2d_switch: code should not be reached > [1] 7151 abort (core dumped) Ok, I see. If sdl2 can support those formats it makes sense to support them, but they must be added to both sdl2_2d_check_format() and sdl2_2d_switch() then. > > There is sdl2_2d_check_format() which reports the supported formats. > > If we hit sdl2_2d_switch() with a format not whitelisted by > > sdl2_2d_check_format() we have a bug somewhere in qemu ... > > I suppose the other solution would be for virtio_gpu_set_scanout() to > check whether the resource's format can actually be used for that > display. Yes, that must be added (independent from the sdl2 patch). > Or in virtio_gpu_resource_create_2d(), I don't know whether > it's possible to use resources in other formats at all. Not directly, they must be converted (using pixman) then. cheers, Gerd
On Mon, Oct 08, 2018 at 08:50:13PM +0200, Max Reitz wrote: > There are some 2D resource formats that can be used through virtio-gpu, > but which are not supported by SDL2 when used for a scanout; these are > all alpha-channel formats and also XBGR (RGBX in non-BE pixman). > > Add these formats in the switch converting pixman to SDL format > constants so a guest cannot crash the VM by triggering the > g_assert_not_reached() with an unsupported format. fixed (sdl2_2d_check_format update) & queued. cheers, Gerd
On 12.10.18 14:48, Gerd Hoffmann wrote: > On Mon, Oct 08, 2018 at 08:50:13PM +0200, Max Reitz wrote: >> There are some 2D resource formats that can be used through virtio-gpu, >> but which are not supported by SDL2 when used for a scanout; these are >> all alpha-channel formats and also XBGR (RGBX in non-BE pixman). >> >> Add these formats in the switch converting pixman to SDL format >> constants so a guest cannot crash the VM by triggering the >> g_assert_not_reached() with an unsupported format. > > fixed (sdl2_2d_check_format update) & queued. Thanks! Max
diff --git a/ui/sdl2-2d.c b/ui/sdl2-2d.c index 85484407be..c9df72636a 100644 --- a/ui/sdl2-2d.c +++ b/ui/sdl2-2d.c @@ -101,15 +101,24 @@ void sdl2_2d_switch(DisplayChangeListener *dcl, case PIXMAN_r5g6b5: format = SDL_PIXELFORMAT_RGB565; break; + case PIXMAN_a8r8g8b8: case PIXMAN_x8r8g8b8: format = SDL_PIXELFORMAT_ARGB8888; break; + case PIXMAN_a8b8g8r8: + case PIXMAN_x8b8g8r8: + format = SDL_PIXELFORMAT_ABGR8888; + break; + case PIXMAN_r8g8b8a8: case PIXMAN_r8g8b8x8: format = SDL_PIXELFORMAT_RGBA8888; break; case PIXMAN_b8g8r8x8: format = SDL_PIXELFORMAT_BGRX8888; break; + case PIXMAN_b8g8r8a8: + format = SDL_PIXELFORMAT_BGRA8888; + break; default: g_assert_not_reached(); }
There are some 2D resource formats that can be used through virtio-gpu, but which are not supported by SDL2 when used for a scanout; these are all alpha-channel formats and also XBGR (RGBX in non-BE pixman). Add these formats in the switch converting pixman to SDL format constants so a guest cannot crash the VM by triggering the g_assert_not_reached() with an unsupported format. Signed-off-by: Max Reitz <mreitz@redhat.com> --- ui/sdl2-2d.c | 9 +++++++++ 1 file changed, 9 insertions(+)