[Bug 228755] libvgl under syscons causes system reboot (via SDL 1.2)

[Bruce Evans]

On Tue, 5 Jun 2018, Bruce Evans wrote:

> On Mon, 4 Jun 2018 a bug that doesn't want replies (especially to the 
> reporter)@freebsd.org wrote:
>
>> https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=228755
>> 
>>            Bug ID: 228755
>>           Summary: libvgl under syscons causes system reboot (via SDL
>>                    1.2)
>> ...
>
> Support for libvgl seemed to be quite broken.  Its demo works surprisingly
> well under FreeBSD-5 (it supports many graphics modes that syscons didn't
> support under FreeBSD), but under -current its demo never works.  I debugged
> this a bit further today.  The breakage seems to be small.  libvgl still
> calls mmap() with flags MAP_FILE, but that was never valid and now causes
> mmap() to fail.  MAP_FILE is the non-flag 0.  One of the flags MAP_SHARED
> or MAP_PRIVATE must be set unless the OS release is < P_OSREL_MAP_FSTRICT =
> 11000036.  So libvgl's misuse of mmap() apparently became fatal in 
> FreeBSD-11.

P_OSREL_MAP_FSTRICT is r271724.

> When mmap() fails, libvgl cleans up.  The failure usually occurs after a
> successful mode switch, so considerable cleanup is needed.  This always
> works here.  But the demo does no error checking and crashes with SIGBUS.
> It is hard to see how this could cause a reboot.
>
> After fixing libvgl to use MAP_SHARED, libvgl and the demo work much the
> same as in FreeBSD-5.  Modes up to 1280x1024x8 work in both.  Fancier
> modes fail in both.  The only interesting difference is that mmap() fails
> for VGA mode 257 in -current both works in FreeBSD-5.  The same (FreeBSD-5)
> libvgl and demo program apparently calculates a too-large frame buffer size
> in -current only, and mmapping this fails.  mmap() also fails for
> 1920x1080x8.  Getting this size wrong in the driver is more likely to
> cause a reboot than failing.

This is the driver getting the check wrong for all frame buffer sizes that
are not a multiple of PAGE_SIZE.  The frame buffer size is not a multiple
of PAGE SIZE mainly for the interesting geometries 800x600x8 and
1920x1080x8.  I use this fix:

Index: vesa.c
===================================================================
--- vesa.c	(revision 334595)
+++ vesa.c	(working copy)
@@ -1642,7 +1642,7 @@
  	    (adp->va_info.vi_flags & V_INFO_LINEAR) != 0) {
  		/* va_window_size == va_buffer_size/vi_planes */
  		/* XXX: is this correct? */
-		if (offset > adp->va_window_size - PAGE_SIZE)
+		if (offset > trunc_page(adp->va_window_size))
  			return (-1);
  		*paddr = adp->va_info.vi_buffer + offset;
  #ifdef VM_MEMATTR_WRITE_COMBINING

After fixing this, even 1920x1080x8 works.

More serious bugs are possible by getting this wrong.  FreeBSD-5 has
the same buggy range check (only in vga.c, the same as now, but it
doesn't matter there because the frame buffer size for old vga modes
is a small multiple of 32K).  Mode 259 only works in FreeBSD-5 because
the frame buffer size for all vesa modes is the hardware size (128MB
for old Radeon here) and this is a multiple of PAGE_SIZE.  Hopefully
the map is limited to the hardware frame buffer.

However, mode switching seems to have been seriously broken by related
changes.  In FreeBSD-5, the mapping was fairly static (established by
the BIOS and just looked up by the kernel?)  In -current, mode switches
for vesa (but not vga) modes call pmap_mapdev*() and pmap_unmapdev*()
to change the mapping.  This functions call kva_alloc() and kva_free().
This is fragile even for the usual case of mode switches in the keyboard
interrupt handler and broken for mode switches in ddb.  Most mode
switches are for vty switches to a vty in a different mode.

Bruce