Page MenuHomeFreeBSD

bhyve: Validate host PAs used to map passthrough BARs.
ClosedPublic

Authored by jhb on Aug 16 2022, 11:03 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, May 10, 5:50 AM
Unknown Object (File)
Feb 16 2024, 8:19 AM
Unknown Object (File)
Feb 16 2024, 8:19 AM
Unknown Object (File)
Feb 16 2024, 8:19 AM
Unknown Object (File)
Feb 16 2024, 8:19 AM
Unknown Object (File)
Feb 16 2024, 4:40 AM
Unknown Object (File)
Feb 14 2024, 10:53 AM
Unknown Object (File)
Feb 13 2024, 11:36 PM
Subscribers

Details

Summary

Reject attempts to map host physical address ranges that are not
subsets of a passthrough device's BAR into a guest.

Sponsored by: The FreeBSD Foundation

Diff Detail

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

Event Timeline

This is an alternative approach to D36146. We probably want this version no matter what to MFC, but due to the issue with the PCI BAR I/O ioctls D36146 needs further work anyway, so perhaps it's worth just doing this for now and using the other review as a way to further lock things down.

Ah yes, I like this for MFC purposes.

sys/amd64/vmm/io/ppt.c
476

I think we also want to round len up to the next multiple of the system page size (and probably reject mappings of length zero?). I believe vm_map_find(), called from vm_map_mmio(), effectively assumes that the size of the mapping has been rounded up by the caller.

sys/amd64/vmm/io/ppt.c
476

Hmmm, some of that falls out by having the mapping be a subset of a BAR I think. But given we plan to establish EPT mappings with this, I think we can perhaps do a stronger check and require hpa to be page aligned and len to be a positive multiple of PAGE_SIZE.

  • Add additional checks on hpa and len for PAGE_SIZE alignment.
sys/amd64/vmm/io/ppt.c
476

It's possible to have sub-page BARs though, so I think we have to handle the possibility that len < PAGE_SIZE, no? I'm not sure if we might ever encounter BARs larger than a page with size not a multiple of the page size.

sys/amd64/vmm/io/ppt.c
476

We can't map such BARs safely into EPT tables though. We would have to force all I/O access to be trapped and indirected as we can't safely expose the other part of that host page (which might hold registers for some other PCI device on the host) to the guest.

sys/amd64/vmm/io/ppt.c
476

FWIW, bhyve(8) already rejects BARs that are not multiples of a page size:

if (bartype != PCIBAR_IO) {
        if (((base | size) & PAGE_MASK) != 0) {
                warnx("passthru device %d/%d/%d BAR %d: "
                    "base %#lx or size %#lx not page aligned\n",
                    sc->psc_sel.pc_bus, sc->psc_sel.pc_dev,
                    sc->psc_sel.pc_func, i, base, size);
                return (-1);
        }
}
markj added inline comments.
sys/amd64/vmm/io/ppt.c
468

I suspect we should apply the same constraints to gpa. That is, gpa should be page-aligned (or at least truncated to a page boundary), and we should check that gpa + len doesn't overflow.

476

Oh. I didn't see that. I think I was conflating that scenario with the case I had to deal with where the PBA and MSI-X table can overlap within a page.

This revision is now accepted and ready to land.Aug 18 2022, 6:51 PM
sys/amd64/vmm/io/ppt.c
468

Yes, and it occurs to me that this probably needs a comment that this is really about EPT page size and not host page size (but that those are identical in practice).