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
F108469141: D38127.diff
Sat, Jan 25, 4:50 AM
Unknown Object (File)
Fri, Jan 3, 11:38 PM
Unknown Object (File)
Dec 9 2024, 1:26 AM
Unknown Object (File)
Nov 16 2024, 2:49 PM
Unknown Object (File)
Nov 16 2024, 2:20 PM
Unknown Object (File)
Nov 16 2024, 12:22 PM
Unknown Object (File)
Oct 4 2024, 2:27 PM
Unknown Object (File)
Sep 26 2024, 6:27 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.)