Changeset View
Standalone View
usr.sbin/bhyve/bhyverun.c
Context not available. | |||||
__FBSDID("$FreeBSD$"); | __FBSDID("$FreeBSD$"); | ||||
#include <sys/types.h> | #include <sys/types.h> | ||||
#include <sys/capsicum.h> | |||||
#include <sys/cpuset.h> | |||||
#include <sys/mman.h> | #include <sys/mman.h> | ||||
#include <sys/time.h> | #include <sys/time.h> | ||||
Context not available. | |||||
#include <stdlib.h> | #include <stdlib.h> | ||||
#include <string.h> | #include <string.h> | ||||
#include <err.h> | #include <err.h> | ||||
#include <errno.h> | |||||
#include <libgen.h> | #include <libgen.h> | ||||
#include <nl_types.h> | |||||
#include <unistd.h> | #include <unistd.h> | ||||
#include <assert.h> | #include <assert.h> | ||||
#include <errno.h> | #include <errno.h> | ||||
Context not available. | |||||
#include <pthread_np.h> | #include <pthread_np.h> | ||||
#include <sysexits.h> | #include <sysexits.h> | ||||
#include <stdbool.h> | #include <stdbool.h> | ||||
#include <termios.h> | |||||
#include <machine/vmm.h> | #include <machine/vmm.h> | ||||
#include <machine/vmm_dev.h> | |||||
grehan: This is in capsicum_helpers.h so no need to be included again. | |||||
#include <vmmapi.h> | #include <vmmapi.h> | ||||
#include "bhyverun.h" | #include "bhyverun.h" | ||||
Context not available. | |||||
struct vmctx *ctx; | struct vmctx *ctx; | ||||
int error; | int error; | ||||
bool reinit, romboot; | bool reinit, romboot; | ||||
cap_rights_t rights; | |||||
u_long cmds[] = {VM_RUN, VM_SUSPEND, VM_REINIT, VM_ALLOC_MEMSEG, | |||||
emasteUnsubmitted Done Inline ActionsDo these follow the same order as somewhere else? It's probably worth referencing the list to aid someone comparing this list with the set of VM_ ioctls in the future. emaste: Do these follow the same order as somewhere else? It's probably worth referencing the list to… | |||||
VM_GET_MEMSEG, VM_MMAP_MEMSEG, VM_MMAP_MEMSEG, VM_MMAP_GETNEXT, | |||||
VM_SET_REGISTER, VM_GET_REGISTER, VM_SET_SEGMENT_DESCRIPTOR, | |||||
VM_GET_SEGMENT_DESCRIPTOR, VM_INJECT_EXCEPTION, VM_LAPIC_IRQ, | |||||
VM_LAPIC_LOCAL_IRQ, VM_LAPIC_MSI, VM_IOAPIC_ASSERT_IRQ, | |||||
Not Done Inline ActionsCan this definition be moved into libvmmapi ? Easier to keep in sync then. grehan: Can this definition be moved into libvmmapi ? Easier to keep in sync then.
| |||||
Not Done Inline ActionsWhy? It's private variable, used only by this one function, the ioctls are defined outside libvmmapi, and I don't see any benefits to make it publicly available from library. kaktus: Why? It's private variable, used only by this one function, the ioctls are defined outside… | |||||
Not Done Inline Actions... or given that I maintain bhyve I have a stake in making sure that abstractions are kept. There are no direct VM_* calls in bhyve, and instead all of these are wrapped in libvmmapi so that the implementation can be changed if need be. There have already been modifications to ioctl values where bhyve didn't have to change. Stepping around this just for capsicum is a step backwards, hence my proposal to have an api call in libvmmapi to return the list of ioctl cmds. grehan: ... or given that I maintain bhyve I have a stake in making sure that abstractions are kept. | |||||
VM_IOAPIC_DEASSERT_IRQ, VM_IOAPIC_PULSE_IRQ, VM_IOAPIC_PINCOUNT, | |||||
VM_ISA_ASSERT_IRQ, VM_ISA_DEASSERT_IRQ, VM_ISA_PULSE_IRQ, | |||||
VM_ISA_SET_IRQ_TRIGGER, VM_SET_CAPABILITY, VM_GET_CAPABILITY, | |||||
VM_BIND_PPTDEV, VM_UNBIND_PPTDEV, VM_MAP_PPTDEV_MMIO, VM_PPTDEV_MSI, | |||||
VM_PPTDEV_MSIX, VM_INJECT_NMI, VM_STATS, VM_STAT_DESC, | |||||
VM_SET_X2APIC_STATE, VM_GET_X2APIC_STATE, VM_GET_HPET_CAPABILITIES, | |||||
VM_GET_GPA_PMAP, VM_GLA2GPA, VM_ACTIVATE_CPU, VM_GET_CPUS, | |||||
VM_SET_INTINFO, VM_GET_INTINFO, VM_RTC_WRITE, VM_RTC_READ, | |||||
VM_RTC_SETTIME, VM_RTC_GETTIME, VM_RESTART_INSTRUCTION}; | |||||
reinit = romboot = false; | reinit = romboot = false; | ||||
Not Done Inline ActionsI think this one should be just CAP_MMAP_RW (no executable permission needed). grehan: I think this one should be just CAP_MMAP_RW (no executable permission needed). | |||||
Not Done Inline ActionsYou're right. It was leftover from some initial version. kaktus: You're right. It was leftover from some initial version. | |||||
Context not available. | |||||
exit(1); | exit(1); | ||||
} | } | ||||
cap_rights_init(&rights, CAP_IOCTL, CAP_MMAP_RWX); | |||||
if (cap_rights_limit(ctx->fd, &rights) == -1 && errno != ENOSYS) | |||||
errx(EX_OSERR, "Unable to apply rights for sandbox"); | |||||
if (cap_ioctls_limit(STDIN_FILENO, cmds, nitems(cmds)) == -1 && errno != ENOSYS) | |||||
errx(EX_OSERR, "Unable to apply rights for sandbox"); | |||||
if (reinit) { | if (reinit) { | ||||
error = vm_reinit(ctx); | error = vm_reinit(ctx); | ||||
if (error) { | if (error) { | ||||
Not Done Inline ActionsYou could combine this into an if (cap_rights_limit() || cap_ioctls_limit()) if desired emaste: You could combine this into an `if (cap_rights_limit() || cap_ioctls_limit())` if desired | |||||
Context not available. | |||||
uint64_t rip; | uint64_t rip; | ||||
size_t memsize; | size_t memsize; | ||||
char *optstr; | char *optstr; | ||||
#ifdef notyet | |||||
cap_rights_t rights; | |||||
u_long cmds[] = {TIOCGETA, TIOCSETA, TIOCGWINSZ}; | |||||
#endif | |||||
lattera-gmail.comUnsubmitted Done Inline ActionsWhy not yet? lattera-gmail.com: Why not yet? | |||||
kaktusAuthorUnsubmitted Done Inline Actionsstdio limits require some more planning to assure we're limiting the correct fd (i.e. stdio or nmdm etc) kaktus: stdio limits require some more planning to assure we're limiting the correct fd (i.e. stdio or… | |||||
kaktusAuthorUnsubmitted Not Done Inline Actionsstdin/out/err are now limited. kaktus: stdin/out/err are now limited. | |||||
bvmcons = 0; | bvmcons = 0; | ||||
progname = basename(argv[0]); | progname = basename(argv[0]); | ||||
Context not available. | |||||
if (lpc_bootrom()) | if (lpc_bootrom()) | ||||
fwctl_init(); | fwctl_init(); | ||||
catopen("libc", NL_CAT_LOCALE); | |||||
#ifdef notyet | |||||
Not Done Inline ActionsNo need to check ENOSYS. oshogbo: No need to check ENOSYS. | |||||
cap_rights_init(&rights, CAP_READ, CAP_FCNTL, CAP_FSTAT, CAP_IOCTL, CAP_EVENT); | |||||
if (cap_rights_limit(STDIN_FILENO, &rights) == -1 && errno != ENOSYS) | |||||
errx(EX_OSERR, "Unable to apply rights for sandbox"); | |||||
if (cap_ioctls_limit(STDIN_FILENO, cmds, nitems(cmds)) == -1 && errno != ENOSYS) | |||||
errx(EX_OSERR, "Unable to apply rights for sandbox!"); | |||||
Not Done Inline ActionsNo need for this warning. oshogbo: No need for this warning. | |||||
cap_rights_clear(&rights, CAP_READ); | |||||
cap_rights_set(&rights, CAP_WRITE); | |||||
if (cap_rights_limit(STDOUT_FILENO, &rights) == -1 && errno != ENOSYS) | |||||
errx(EX_OSERR, "Unable to apply rights for sandbox"); | |||||
if (cap_ioctls_limit(STDOUT_FILENO, cmds, nitems(cmds)) == -1 && errno != ENOSYS) | |||||
errx(EX_OSERR, "Unable to apply rights for sandbox"); | |||||
if (cap_rights_limit(STDERR_FILENO, &rights) == -1 && errno != ENOSYS) | |||||
errx(EX_OSERR, "Unable to apply rights for sandbox"); | |||||
if (cap_ioctls_limit(STDERR_FILENO, cmds, nitems(cmds)) == -1 && errno != ENOSYS) | |||||
errx(EX_OSERR, "Unable to apply rights for sandbox"); | |||||
#endif | |||||
if (cap_enter() == -1) { | |||||
if (errno == ENOSYS) | |||||
warn("Capsicum unavailable, running without a sandbox"); | |||||
else | |||||
errx(EX_OSERR, "cap_enter() failed"); | |||||
} | |||||
/* | /* | ||||
* Change the proc title to include the VM name. | * Change the proc title to include the VM name. | ||||
*/ | */ | ||||
Context not available. | |||||
Not Done Inline ActionsAs an aside, I dislike the common errno != ENOSYS that we've been propagating through many Capsicumized applications. I like the way you've done this; this warning is an improvement over the common pattern. I'd like to move to using something like your #ifndef WITHOUT_CAPSICUM universally, including setting it in the FreeBSD build infrastructure, and then remove the ENOSYS cases. emaste: As an aside, I dislike the common `errno != ENOSYS` that we've been propagating through many… | |||||
Not Done Inline ActionsYes, the errno != ENOSYS is a bit verbose and I'd like to see something to replace it but this is out of scope of this patch anyway. kaktus: Yes, the errno != ENOSYS is a bit verbose and I'd like to see something to replace it but this… |
This is in capsicum_helpers.h so no need to be included again.