Page MenuHomeFreeBSD

bhyve: Fix a global buffer overread in the PCI hda device model.
ClosedPublic

Authored by jhb on Jan 19 2023, 10:40 PM.
Tags
None
Referenced Files
Unknown Object (File)
Dec 23 2023, 1:10 AM
Unknown Object (File)
Dec 12 2023, 5:21 PM
Unknown Object (File)
Nov 26 2023, 5:21 AM
Unknown Object (File)
Nov 24 2023, 1:00 PM
Unknown Object (File)
Nov 24 2023, 5:54 AM
Unknown Object (File)
Nov 22 2023, 1:36 PM
Unknown Object (File)
Nov 22 2023, 11:58 AM
Unknown Object (File)
Nov 18 2023, 12:31 AM

Details

Summary

hda_write did not validate the relative register offset before using
it as an index into the hda_set_reg_table array to lookup a function
pointer to execute after updating the register's value.

PR: 264435
Reported by: Robert Morris <rtm@lcs.mit.edu>
Sponsored by: The FreeBSD Foundation

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

jhb requested review of this revision.Jan 19 2023, 10:40 PM
emaste added inline comments.
usr.sbin/bhyve/pci_hda.c
716

offset is "enforced" to be < HDA_LAST_OFFSET via an assertion in hda_get_reg_by_offset; (as a subsequent change) should we turn that into an error return?

This revision is now accepted and ready to land.Jan 20 2023, 12:55 AM
corvink added a subscriber: corvink.
corvink added inline comments.
usr.sbin/bhyve/pci_hda.c
716

I'd prefer that.

usr.sbin/bhyve/pci_hda.c
716

No, the offset is never bigger than that since the BAR is registered as having that size:

static int
pci_hda_init(struct pci_devinst *pi, nvlist_t *nvl)
{
...
	/* allocate one BAR register for the Memory address offsets */
	pci_emul_alloc_bar(pi, 0, PCIBAR_MEM32, HDA_LAST_OFFSET);

The offset should never be larger unless there is a bug in pci_emul.c, so using an assert() rather than an error check in the get/set_reg routines is correct. (If we break generic PCI BAR handling in bhyve, we'd rather get core dumps due to assertion errors, not silently discarded errors.)