Differential D8290 Diff 24551 usr.sbin/bhyve/bhyverun.c

Changeset View

Standalone View

usr.sbin/bhyve/bhyverun.c

Show All 24 Lines
*		*
* $FreeBSD$		* $FreeBSD$
*/		*/

#include <sys/cdefs.h>		#include <sys/cdefs.h>
__FBSDID("$FreeBSD$");		__FBSDID("$FreeBSD$");

#include <sys/types.h>		#include <sys/types.h>
		#ifndef WITHOUT_CAPSICUM
		#include <sys/capsicum.h>
		#endif
#include <sys/mman.h>		#include <sys/mman.h>
#include <sys/time.h>		#include <sys/time.h>

#include <machine/atomic.h>		#include <machine/atomic.h>
#include <machine/segments.h>		#include <machine/segments.h>

		#ifndef WITHOUT_CAPSICUM
		#include <capsicum_helpers.h>
		#endif
#include <stdio.h>		#include <stdio.h>
#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 <unistd.h>		#include <unistd.h>
#include <assert.h>		#include <assert.h>
#include <errno.h>		#include <errno.h>
#include <pthread.h>		#include <pthread.h>
#include <pthread_np.h>		#include <pthread_np.h>
#include <sysexits.h>		#include <sysexits.h>
#include <stdbool.h>		#include <stdbool.h>
		#include <termios.h>
		grehanUnsubmitted Not Done Inline Actions This is in capsicum_helpers.h so no need to be included again. grehan: This is in capsicum_helpers.h so no need to be included again.

#include <machine/vmm.h>		#include <machine/vmm.h>
		#ifndef WITHOUT_CAPSICUM
		#include <machine/vmm_dev.h>
		#endif
#include <vmmapi.h>		#include <vmmapi.h>

#include "bhyverun.h"		#include "bhyverun.h"
#include "acpi.h"		#include "acpi.h"
#include "atkbdc.h"		#include "atkbdc.h"
#include "inout.h"		#include "inout.h"
#include "dbgport.h"		#include "dbgport.h"
#include "fwctl.h"		#include "fwctl.h"
▲ Show 20 Lines • Show All 636 Lines • ▼ Show 20 Lines	if (err) {
exit(1);		exit(1);
}		}

vm_set_capability(ctx, cpu, VM_CAP_ENABLE_INVPCID, 1);		vm_set_capability(ctx, cpu, VM_CAP_ENABLE_INVPCID, 1);
}		}

static struct vmctx *		static struct vmctx *
do_open(const char *vmname)		do_open(const char *vmname)
{		{
		emasteUnsubmitted Done Inline Actions Do 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…
struct vmctx *ctx;		struct vmctx *ctx;
int error;		int error;
bool reinit, romboot;		bool reinit, romboot;
		#ifndef WITHOUT_CAPSICUM
		grehanUnsubmitted Not Done Inline Actions Can 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.
		kaktusAuthorUnsubmitted Not Done Inline Actions Why? 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. If you know something that I don't or just simply want it moved please state your opinion clearly and I'll follow :-) kaktus: Why? It's private variable, used only by this one function, the ioctls are defined outside…
		grehanUnsubmitted 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.
		cap_rights_t rights;
		const u_long *cmds;
		size_t ncmds;
		#endif

reinit = romboot = false;		reinit = romboot = false;

if (lpc_bootrom())		if (lpc_bootrom())
romboot = true;		romboot = true;

error = vm_create(vmname);		error = vm_create(vmname);
if (error) {		if (error) {
Show All 22 Lines	#endif
}		}

ctx = vm_open(vmname);		ctx = vm_open(vmname);
if (ctx == NULL) {		if (ctx == NULL) {
perror("vm_open");		perror("vm_open");
exit(1);		exit(1);
}		}

		#ifndef WITHOUT_CAPSICUM
		cap_rights_init(&rights, CAP_IOCTL, CAP_MMAP_RWX);
		grehanUnsubmitted Not Done Inline Actions I 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).
		kaktusAuthorUnsubmitted Not Done Inline Actions You're right. It was leftover from some initial version. kaktus: You're right. It was leftover from some initial version.
		if (cap_rights_limit(vm_get_device_fd(ctx), &rights) == -1 &&
		errno != ENOSYS)
		errx(EX_OSERR, "Unable to apply rights for sandbox");
		vm_get_ioctls(&ncmds);
		cmds = vm_get_ioctls(NULL);
		if (cmds == NULL)
		emasteUnsubmitted Not Done Inline Actions You 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
		errx(EX_OSERR, "out of memory");
		if (cap_ioctls_limit(vm_get_device_fd(ctx), cmds, ncmds) == -1 &&
		errno != ENOSYS)
		errx(EX_OSERR, "Unable to apply rights for sandbox");
		free((u_long *)cmds);
		#endif

if (reinit) {		if (reinit) {
error = vm_reinit(ctx);		error = vm_reinit(ctx);
if (error) {		if (error) {
perror("vm_reinit");		perror("vm_reinit");
exit(1);		exit(1);
}		}
}		}
return (ctx);		return (ctx);
}		}

int		int
main(int argc, char *argv[])		main(int argc, char *argv[])
{		{
int c, error, gdb_port, err, bvmcons;		int c, error, gdb_port, err, bvmcons;
int max_vcpus, mptgen, memflags;		int max_vcpus, mptgen, memflags;
int rtc_localtime;		int rtc_localtime;
struct vmctx *ctx;		struct vmctx *ctx;
uint64_t rip;		uint64_t rip;
size_t memsize;		size_t memsize;
char *optstr;		char *optstr;
		lattera-gmail.comUnsubmitted Done Inline Actions Why not yet? lattera-gmail.com: Why not yet?
		kaktusAuthorUnsubmitted Done Inline Actions stdio 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 Actions stdin/out/err are now limited. kaktus: stdin/out/err are now limited.

bvmcons = 0;		bvmcons = 0;
progname = basename(argv[0]);		progname = basename(argv[0]);
gdb_port = 0;		gdb_port = 0;
guest_ncpus = 1;		guest_ncpus = 1;
memsize = 256 * MB;		memsize = 256 * MB;
mptgen = 1;		mptgen = 1;
rtc_localtime = 1;		rtc_localtime = 1;
▲ Show 20 Lines • Show All 171 Lines • ▼ Show 20 Lines	main(int argc, char *argv[])

if (acpi) {		if (acpi) {
error = acpi_build(ctx, guest_ncpus);		error = acpi_build(ctx, guest_ncpus);
assert(error == 0);		assert(error == 0);
}		}

if (lpc_bootrom())		if (lpc_bootrom())
fwctl_init();		fwctl_init();

		#ifndef WITHOUT_CAPSICUM
		caph_cache_catpages();

		if ((caph_limit_stdout() == -1 \|\| caph_limit_stderr() == -1) && errno != ENOSYS)
		oshogboUnsubmitted Not Done Inline Actions No need to check ENOSYS. oshogbo: No need to check ENOSYS.
		errx(EX_OSERR, "Unable to apply rights for sandbox");

		if (cap_enter() == -1) {
		if (errno != ENOSYS)
		errx(EX_OSERR, "cap_enter() failed");
		warn("Capsicum unavailable, running without a sandbox");
		oshogboUnsubmitted Not Done Inline Actions No need for this warning. oshogbo: No need for this warning.
		}
		#endif

/*		/*
* Change the proc title to include the VM name.		* Change the proc title to include the VM name.
*/		*/
setproctitle("%s", vmname);		setproctitle("%s", vmname);

/*		/*
* Add CPU 0		* Add CPU 0
*/		*/
fbsdrun_addcpu(ctx, BSP, BSP, rip);		fbsdrun_addcpu(ctx, BSP, BSP, rip);

/*		/*
* Head off to the main event dispatch loop		* Head off to the main event dispatch loop
*/		*/
mevent_dispatch();		mevent_dispatch();
		emasteUnsubmitted Not Done Inline Actions As 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…
		kaktusAuthorUnsubmitted Not Done Inline Actions Yes, 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…

exit(1);		exit(1);
}		}