In bhyve's pci_nvme_append_iov_req(), if the guest provides a zero prp1 (i.e. gpaddr) for an I/O operation, then this code runs: if ((req->prev_gpaddr + req->prev_size) == gpaddr) { iovidx = req->io_req.br_iovcnt - 1; ...; req->prev_size += size; ...; req->io_req.br_iov[iovidx].iov_len = req->prev_size; prev_gpaddr, prev_size, and br_iovcnt are all ordinarily zero at this point. So iovidx = -1, and the assignment to br_iov[iovidx].iov_len actually overwrites io_req.br_param. This later causes a bad pointer dereference in pci_nvme_io_done(): struct pci_nvme_ioreq *req = br->br_param; struct nvme_submission_queue *sq = req->nvme_sq; You can see this happen if you boot a FreeBSD guest kernel in bhyve with an nvme device, after modifying the guest kernel's nvme_payload_map() in /sys/dev/nvme/nvme_qpair.c to set tr->req->cmd.prp1 to zero when it is called for the third time.
Created attachment 234217 [details] Check existance of previous IOV before concatenating
Can you try the attached? Verified an NVMe based VM can still boot but haven't tried your case.
(In reply to Chuck Tuffli from comment #2) Your patch makes the problem go away for me.
Review up https://reviews.freebsd.org/D35328
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=88951aaaee73b87121b0f121224fe188a5b5e6e3 commit 88951aaaee73b87121b0f121224fe188a5b5e6e3 Author: Chuck Tuffli <chuck@FreeBSD.org> AuthorDate: 2022-06-09 18:19:32 +0000 Commit: Chuck Tuffli <chuck@FreeBSD.org> CommitDate: 2022-08-13 19:16:02 +0000 bhyve nvme: Fix out-of-bound IOV array access Summary: NVMe operations indicate the memory region(s) associated with a command via physical region pages (PRPs). Since each PRP has a fixed size, contiguous memory regions larger than the PRP size require multiple PRP entries. Instead of issuing a blockif call for each PRP, the NVMe emulation concatenates multiple contiguous PRP entries into a single blockif request. The test for contiguous regions has a bug such that it mistakenly treats an initial PRP address of zero as a contiguous range and concatenates it with the previous. But because there is no previous IOV, the concatenation code corrupts the IO request structure and leads to a segmentation fault when the blockif request completes. Fix is to test for the existence of a previous range before trying to concatenate the current range with the previous one. While in the area, rename pci_nvme_append_iov_req()'s lba parameter to offset to match its usage. PR: 264177 Reported by: Robert Morris <rtm@lcs.mit.edu> Reviewed by: jhb MFC after: 2 weeks Differential Revision: https://reviews.freebsd.org/D35328 usr.sbin/bhyve/pci_nvme.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-)
A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=a00b97054a504792eb6fa028728726f5c37744c3 commit a00b97054a504792eb6fa028728726f5c37744c3 Author: Chuck Tuffli <chuck@FreeBSD.org> AuthorDate: 2022-06-09 18:19:32 +0000 Commit: Chuck Tuffli <chuck@FreeBSD.org> CommitDate: 2022-11-20 01:48:45 +0000 bhyve nvme: Fix out-of-bound IOV array access Summary: NVMe operations indicate the memory region(s) associated with a command via physical region pages (PRPs). Since each PRP has a fixed size, contiguous memory regions larger than the PRP size require multiple PRP entries. Instead of issuing a blockif call for each PRP, the NVMe emulation concatenates multiple contiguous PRP entries into a single blockif request. The test for contiguous regions has a bug such that it mistakenly treats an initial PRP address of zero as a contiguous range and concatenates it with the previous. But because there is no previous IOV, the concatenation code corrupts the IO request structure and leads to a segmentation fault when the blockif request completes. Fix is to test for the existence of a previous range before trying to concatenate the current range with the previous one. While in the area, rename pci_nvme_append_iov_req()'s lba parameter to offset to match its usage. PR: 264177 (cherry picked from commit 88951aaaee73b87121b0f121224fe188a5b5e6e3) usr.sbin/bhyve/pci_nvme.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-)