Bug 264582 - bhyve: hda_send_command() can index beyond the end of sc->codecs[]
Summary: bhyve: hda_send_command() can index beyond the end of sc->codecs[]
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bhyve (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: John Baldwin
URL:
Keywords: needs-patch, needs-qa
Depends on:
Blocks:
 
Reported: 2022-06-09 23:19 UTC by Robert Morris
Modified: 2023-02-01 19:57 UTC (History)
1 user (show)

See Also:
jhb: mfc-stable13+
jhb: mfc-stable12-


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Robert Morris 2022-06-09 23:19:49 UTC
The guest specifies cad as a 4-bit field in verb, and thus can arrange
for cad to be 15:

hda_send_command(struct hda_softc *sc, uint32_t verb)
{
        struct hda_codec_inst *hci = NULL;
        struct hda_codec_class *codec = NULL;
        uint8_t cad = (verb >> HDA_CMD_CAD_SHIFT) & 0x0f;

        hci = sc->codecs[cad];

But codecs[] has length 15 (HDA_CODEC_MAX).
Comment 1 commit-hook freebsd_committer freebsd_triage 2023-01-20 17:59:58 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=cf57f20edcf9c75f0f9f1ac1c44729184970b9d9

commit cf57f20edcf9c75f0f9f1ac1c44729184970b9d9
Author:     John Baldwin <jhb@FreeBSD.org>
AuthorDate: 2023-01-20 17:58:38 +0000
Commit:     John Baldwin <jhb@FreeBSD.org>
CommitDate: 2023-01-20 17:58:38 +0000

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

    The sc->codecs array contains HDA_CODEC_MAX (15) entries.  The
    guest-supplied cad field in the verb provided to hda_send_command is a
    4-bit field that was used as an index into sc->codecs without any
    bounds checking.  The highest value (15) would overflow the array.

    Other uses of sc->codecs in the device model used sc->codecs_no to
    determine which array indices have been initialized, so use a similar
    check to reject requests for uninitialized or invalid cad indices in
    hda_send_command.

    PR:             264582
    Reported by:    Robert Morris <rtm@lcs.mit.edu>
    Reviewed by:    corvink, markj, emaste
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D38128

 usr.sbin/bhyve/pci_hda.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
Comment 2 commit-hook freebsd_committer freebsd_triage 2023-01-26 22:36:06 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=abdc47cd6969a649ee7b4bec0efe0d51bc95dfdb

commit abdc47cd6969a649ee7b4bec0efe0d51bc95dfdb
Author:     John Baldwin <jhb@FreeBSD.org>
AuthorDate: 2023-01-20 17:58:38 +0000
Commit:     John Baldwin <jhb@FreeBSD.org>
CommitDate: 2023-01-26 22:29:06 +0000

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

    The sc->codecs array contains HDA_CODEC_MAX (15) entries.  The
    guest-supplied cad field in the verb provided to hda_send_command is a
    4-bit field that was used as an index into sc->codecs without any
    bounds checking.  The highest value (15) would overflow the array.

    Other uses of sc->codecs in the device model used sc->codecs_no to
    determine which array indices have been initialized, so use a similar
    check to reject requests for uninitialized or invalid cad indices in
    hda_send_command.

    PR:             264582
    Reported by:    Robert Morris <rtm@lcs.mit.edu>
    Reviewed by:    corvink, markj, emaste
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D38128

    (cherry picked from commit cf57f20edcf9c75f0f9f1ac1c44729184970b9d9)

 usr.sbin/bhyve/pci_hda.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
Comment 3 John Baldwin freebsd_committer freebsd_triage 2023-02-01 19:57:09 UTC
Fix merged to stable/13.  Code not present in stable/12.