Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Tale of two hypervisor bugs – Escaping from FreeBSD bhyve

phrack.org

Post reply on HN
Print view View original

31–40 of 47 posts

Re: Tale of two hypervisor bugs – Escaping from FreeBSD bhyve

#31

Post by userbinator »

EvanAnderson wrote: post #12
saagarjha wrote: post #2

> 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…

re: "How about not letting the index overflow..." 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…

I'm fairly certain a real hardware VGA implementation would maintain that index as an unsigned 8-bit value.

I've found an authoritative source - a datasheet for an IBM RAMDAC used for their VGA-compatible video cards:

ftp://retronn.de/docs/pc_hardware/DACs/IBM37RGB524.pdf

    An increment past 0xff will "wrap around" to 0x00.
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...

I haven't looked, but definitely check the "tiny" (512b and below) category if you want to find tricks like these. I'm also reasonably certain there's going to be at least one which does that for a palette animation effect, taking advantage of the wraparound so it won't have to reset the write index again (and thus saving some precious bytes.)

Here's an interesting discussion I found about the behaviour of the DAC ports on actual hardware, also from an emulation perspective (DOSbox):

https://github.com/joncampbell123/dosbox-x/issues/502

Re: Tale of two hypervisor bugs – Escaping from FreeBSD bhyve

#32

Post by landr0id »

cperciva wrote: post #20
landr0id wrote: post #10

Earlier quoted context omitted.

HardenedBSD (FreeBSD fork) has had ASLR and other mitigations since forever. Shawn submitted a patch that was never merged because of mailing list politics or something of that sort + people afraid it was going to break the world. https://reviews.freebsd.org/D473

> politics or something of that sort I suppose you could classify "the patch doesn't work and breaks other things besides" that way.

HardenedBSD got the userland running fine. It was iirc mostly irrational fear by the FreeBSD team that someone's bad application would break.

Re: Tale of two hypervisor bugs – Escaping from FreeBSD bhyve

#33

Post by gonzo »

landr0id wrote: post #32
cperciva wrote: post #20

Earlier quoted context omitted.

> politics or something of that sort I suppose you could classify "the patch doesn't work and breaks other things besides" that way.

HardenedBSD got the userland running fine. It was iirc mostly irrational fear by the FreeBSD team that someone's bad application would break.

That’s not what happened.

Re: Tale of two hypervisor bugs – Escaping from FreeBSD bhyve

#36

Post by de_watcher »

saagarjha wrote: post #28
int_19h wrote: post #23

Earlier quoted context omitted.

This doesn't let the index overflow with the minimum amount of hassle. The behavior of unsigned overflow is well-defined in C, so there's no point writing code with an explicit check that is tricky to do correctly.

I don't know, I think it's kind of messy to rely on overflow behavior to ensure that your element is in bounds. What if the size of the array changes later? There's no clear linkage between the two in my mind.

No, it's natural. The conventions are different in different areas of software development so it may look weird to different groups.

Re: Tale of two hypervisor bugs – Escaping from FreeBSD bhyve

#37

Post by saagarjha »

tonyarkles wrote: post #29
saagarjha wrote: post #28

Earlier quoted context omitted.

I don't know, I think it's kind of messy to rely on overflow behavior to ensure that your element is in bounds. What if the size of the array changes later? There's no clear linkage between the two in my mind.

Keep in mind that this is emulating old hardware. See: http://www.osdever.net/FreeVGA/vga/colorreg.htm If the size of the array changed, it would no longer be compatible with the hardware it's trying to emulate. I understand what you're trying to say and agree in general, but this is a good example of "the exception to the rule".

I guess I’m unhappy with the linkage between “this is an 8-bit value” and “this thing just happens to wrap at 256”; I would be much more satisfied if there was a clear “byte” rationale for the latter as well, which there very well might be but it’s not clear contextually.

To put it differently, I’m fine with relying on the overflow behavior, but only when the semantics have an actual match. For example, I rely on it for the implementations 32-bit operations in a VM I’m writing, but I feel this is OK because the spec for the architecture clearly defines this as the intended behavior and it’s not just “this used to be an int but I realized the array was 256 big so I can abuse an unsigned byte to never have an invalid index”.

Re: Tale of two hypervisor bugs – Escaping from FreeBSD bhyve

#38

Post by saagarjha »

EvanAnderson wrote: post #12
saagarjha wrote: post #2

> 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…

re: "How about not letting the index overflow..." 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…

> The mistake was representing it as anything other than an 8-bit number because that's what the hardware really would have done.

I would be significantly more pleased if this was the documented, intended behavior that was being emulated intentionally, since the previous choice leaves me a bit worried that this is just an instance of “hey we can just use this type as a convenient index that doesn’t go out of bounds”.

Re: Tale of two hypervisor bugs – Escaping from FreeBSD bhyve

#39

Post by saagarjha »

de_watcher wrote: post #36
saagarjha wrote: post #28

Earlier quoted context omitted.

I don't know, I think it's kind of messy to rely on overflow behavior to ensure that your element is in bounds. What if the size of the array changes later? There's no clear linkage between the two in my mind.

No, it's natural. The conventions are different in different areas of software development so it may look weird to different groups.

I’m not sure what group I’d need to be in to think of this patch as natural without qualification…

Re: Tale of two hypervisor bugs – Escaping from FreeBSD bhyve

#40

Post by EvanAnderson »

saagarjha wrote: post #38
EvanAnderson wrote: post #12

Earlier quoted context omitted.

re: "How about not letting the index overflow..." 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…

> The mistake was representing it as anything other than an 8-bit number because that's what the hardware really would have done. I would be significantly more pleased if this was the documented, intended behavior that was being emulated intentionally, since the previous choice leaves me a bit worried that this is just an instance of “hey we can just use this type as a convenient index that doesn’t go out of bounds”.

The sibling comment to yours has a link to such documentation, and a link to a really nice discussion re: reverse engineering this behavior on various clone VGA chipsets. The IBM RAMDAC documentation is pretty authoritative, IMO.
Post reply on HN