Bug 264521 - bhyve: pci_vtscsi_request_handle() can read beyond allocated heap object
Summary: bhyve: pci_vtscsi_request_handle() can read beyond allocated heap object
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-07 15:21 UTC by Robert Morris
Modified: 2023-01-20 18:35 UTC (History)
3 users (show)

See Also:
koobs: mfc-stable13?
koobs: 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-07 15:21:36 UTC
Bhyve's pci_vtscsi_request_handle() says:

        iov_to_buf(iov_in, niov_in, (void **)&cmd_rd);
        ...;
        io->io_hdr.nexus.targ_lun = pci_vtscsi_get_lun(cmd_rd->lun);
        // and further uses of cmd_rd

If the guest driver provided no readable virtio descriptors, then
niov_in will be zero, and iov_to_buf() will cause cmd_rd to point to a
zero-length heap object. If there is a readable descriptor, but it's
smaller than sizeof(cmd_rd), there's also a problem.
Comment 1 Robert Morris 2022-06-07 20:06:47 UTC
Similarly, pci_vtscsi_control_handle() ought to check
bufsize >= sizeof(struct pci_vtscsi_ctrol_tmf) or _an.
Comment 2 John Baldwin freebsd_committer freebsd_triage 2022-08-19 23:43:28 UTC
Potential fix available at https://reviews.FreeBSD.org/D36271 (note that it requires a cosmetic change in D36270 to compile).
Comment 3 Robert Morris 2022-08-20 10:59:47 UTC
(In reply to John Baldwin from comment #2)
Patch D36271 does fix the problem for me.
Comment 4 commit-hook freebsd_committer freebsd_triage 2022-08-29 22:38:26 UTC
A commit in branch main references this bug:

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

commit bb31aee26bd13307d97c5d5bf2b10bf05bdc18fd
Author:     John Baldwin <jhb@FreeBSD.org>
AuthorDate: 2022-08-29 22:36:11 +0000
Commit:     John Baldwin <jhb@FreeBSD.org>
CommitDate: 2022-08-29 22:37:27 +0000

    bhyve virtio-scsi: Avoid out of bounds accesses to guest requests.

    - Ignore I/O requests with insufficiently sized input or output
      buffers (those not containing compete request headers).

    - Ignore control requests with improperly sized buffers.

    - While here, explicitly zero the output header of an I/O request to
      avoid leaking malloc garbage from the host if the header is not
      fully populated.

    PR:             264521
    Reported by:    Robert Morris <rtm@lcs.mit.edu>
    Reviewed by:    mav, emaste
    MFC after:      1 week
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D36271

 usr.sbin/bhyve/pci_virtio_scsi.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)
Comment 5 commit-hook freebsd_committer freebsd_triage 2022-11-11 01:25:29 UTC
A commit in branch stable/13 references this bug:

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

commit b37b564ecf0a2e079acbd1866337a5c6ed739d73
Author:     John Baldwin <jhb@FreeBSD.org>
AuthorDate: 2022-08-29 22:36:11 +0000
Commit:     John Baldwin <jhb@FreeBSD.org>
CommitDate: 2022-11-11 01:10:18 +0000

    bhyve virtio-scsi: Avoid out of bounds accesses to guest requests.

    - Ignore I/O requests with insufficiently sized input or output
      buffers (those not containing compete request headers).

    - Ignore control requests with improperly sized buffers.

    - While here, explicitly zero the output header of an I/O request to
      avoid leaking malloc garbage from the host if the header is not
      fully populated.

    PR:             264521
    Reported by:    Robert Morris <rtm@lcs.mit.edu>
    Reviewed by:    mav, emaste
    MFC after:      1 week
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D36271

    (cherry picked from commit bb31aee26bd13307d97c5d5bf2b10bf05bdc18fd)

 usr.sbin/bhyve/pci_virtio_scsi.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)
Comment 6 commit-hook freebsd_committer freebsd_triage 2022-11-11 01:25:32 UTC
A commit in branch stable/12 references this bug:

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

commit 1282bf40f7b90af1fa90223125e10c8e4edb5c39
Author:     John Baldwin <jhb@FreeBSD.org>
AuthorDate: 2022-08-29 22:36:11 +0000
Commit:     John Baldwin <jhb@FreeBSD.org>
CommitDate: 2022-11-11 01:13:19 +0000

    bhyve virtio-scsi: Avoid out of bounds accesses to guest requests.

    - Ignore I/O requests with insufficiently sized input or output
      buffers (those not containing compete request headers).

    - Ignore control requests with improperly sized buffers.

    - While here, explicitly zero the output header of an I/O request to
      avoid leaking malloc garbage from the host if the header is not
      fully populated.

    PR:             264521
    Reported by:    Robert Morris <rtm@lcs.mit.edu>
    Reviewed by:    mav, emaste
    MFC after:      1 week
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D36271

    (cherry picked from commit bb31aee26bd13307d97c5d5bf2b10bf05bdc18fd)

 usr.sbin/bhyve/pci_virtio_scsi.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)
Comment 7 Ed Maste freebsd_committer freebsd_triage 2023-01-20 18:35:06 UTC
Committed and merged to stable branches