> The below patch fixed the issue: struct {
uint8_t dac_state;
- int dac_rd_index;
- int dac_rd_subindex;
- int dac_wr_index;
- int dac_wr_subindex;
+ uint8_t dac_rd_index;
+ uint8_t dac_rd_subindex;
+ uint8_t dac_wr_index;
+ uint8_t dac_wr_subindex;
uint8_t dac_palette[3 * 256];
uint32_t dac_palette_rgb[256];
} vga_dac;
> The VGA device emulation in bhyve uses 32-bit signed integer as DAC Address Write Mode Register and DAC Address Read Mode Register. These registers are used to access the palette RAM, having 256 entries of intensities for each value of red, green and blue. Data in palette RAM can be read or written by accessing DAC Data Register.> After three successful I/O access to red, green and blue intensity values, DAC Address Write Mode Register or DAC Address Read Mode Register is incremented automatically based on the operation performed. Here is the issue, the values of DAC Address Read Mode Register and DAC Address Write Mode Register does not wrap under index of 256 since the data type is not 'uint8_t', allowing an untrusted guest to read or write past the palette RAM into adjacent heap memory. Ugh, this looks like an ugly patch :( How about not letting the index overflow in the first place? > Though FreeBSD does not have ASLR Why not?! |
I'm fairly certain a real hardware VGA implementation would maintain that index as an unsigned 8-bit value. There's no letting the index overflow, per se. It should overflow back to zero when incremented at 0xFF. The mistake was representing it as anything other than an 8-bit number because that's what the hardware really would have done.
If I had the patience (and a machine close at hand) I'd boot MS-DOS and run thru a little test in DEBUG to see how it acts on a real card to write beyond address 0xFF via the auto-increment feature.
Most of the palette-cycling / setting code I ever wrote just updated the entire palette at once and stopped. I can't think of any particular reason why somebody would want to overflow the DAC write index but it should work. (I suppose there's probably some demoscene code out there somewhere that repeatedly updates the entire palette, outputting to the address write register once then just banging on 0x3c9 repeatedly...)