On Tuesday, February 14, 2017 01:35:59 PM Bartek Rutkowski wrote:
> Author: robak (ports committer)
> Date: Tue Feb 14 13:35:59 2017
> New Revision: 313727
> URL: https://svnweb.freebsd.org/changeset/base/313727
>
> Log:
> Capsicum support for bhyve(8).
>
> Adds Capsicum sandboxing to bhyve.
>
> Submitted by: Pawel Biernacki <[email protected]>
> Reviewed by: grehan, oshogbo
> Approved by: emaste, grehan
> Sponsored by: Mysterious Code Ltd.
> Differential Revision: https://reviews.freebsd.org/D8290
Neat, though it adds a bit more work to future changes. (I have WIP to add
a GDB server to bhyve that adds more ioctls, etc. I can probably just
compile without capsicum though until the set of ioctls stabilizes.)
> Modified: head/lib/libvmmapi/vmmapi.c
> ==============================================================================
> --- head/lib/libvmmapi/vmmapi.c Tue Feb 14 04:52:24 2017
> (r313726)
> +++ head/lib/libvmmapi/vmmapi.c Tue Feb 14 13:35:59 2017
> (r313727)
> @@ -1416,3 +1416,45 @@ vm_restart_instruction(void *arg, int vc
>
> return (ioctl(ctx->fd, VM_RESTART_INSTRUCTION, &vcpu));
> }
> +
> +int
> +vm_get_device_fd(struct vmctx *ctx)
> +{
> +
> + return (ctx->fd);
> +}
> +
> +const cap_ioctl_t *
> +vm_get_ioctls(size_t *len)
> +{
> + cap_ioctl_t *cmds;
> + /* keep in sync with machine/vmm_dev.h */
> + static const cap_ioctl_t vm_ioctl_cmds[] = { VM_RUN, VM_SUSPEND,
> VM_REINIT,
> + VM_ALLOC_MEMSEG, 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, 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 };
> +
> + if (len == NULL) {
> + cmds = malloc(sizeof(vm_ioctl_cmds));
> + if (cmds == NULL)
> + return (NULL);
> + bcopy(vm_ioctl_cmds, cmds, sizeof(vm_ioctl_cmds));
> + return (cmds);
Given you are returning a const array, why do you have to malloc a separate
copy vs just returning vm_ioctl_cmds[] directly?
It would seem that the interface for this could be that it always returns
vm_ioctl_cmds and sets *len if len is not NULL.
> Modified: head/usr.sbin/bhyve/bhyverun.c
> ==============================================================================
> --- head/usr.sbin/bhyve/bhyverun.c Tue Feb 14 04:52:24 2017
> (r313726)
> +++ head/usr.sbin/bhyve/bhyverun.c Tue Feb 14 13:35:59 2017
> (r313727)
> @@ -744,6 +759,21 @@ do_open(const char *vmname)
> exit(1);
> }
>
> +#ifndef WITHOUT_CAPSICUM
> + cap_rights_init(&rights, CAP_IOCTL, CAP_MMAP_RW);
> + 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)
> + 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((cap_ioctl_t *)cmds);
> +#endif
> +
I wonder if instead of doing this in the executable and exposing
vm_get_ioctls, etc. if the API shouldn't really be something like
'vm_limit_rights(ctx)' and this code should be in that function in
libvmmapi? It would be something like (assuming you drop
vm_get_ioctls() entirely and expose vm_ioctl_cmds[] as a static
global in the library):
int
vm_limit_rights(struct vm_ctx *ctx)
{
...
cap_rights_init(&rights, CAP_IOCTL, CAP_MMAP_RW);
if (cap_rights_limit(ctx->fd, &rights) == -1) {
if (errno == ENOSYS)
return (0);
else
return (-1);
}
/* Shouldn't get ENOSYS here if cap_rights_limit worked. */
return (cap_ioctls_limit(ctx->fd, vm_ioctl_cmds, nitems(vm_ioctl_cmds));
}
You wouldn't need vm_get_device_fd() either in this API.
> Modified: head/usr.sbin/bhyve/consport.c
> ==============================================================================
> --- head/usr.sbin/bhyve/consport.c Tue Feb 14 04:52:24 2017
> (r313726)
> +++ head/usr.sbin/bhyve/consport.c Tue Feb 14 13:35:59 2017
> (r313727)
> @@ -123,6 +133,13 @@ console_handler(struct vmctx *ctx, int v
> return (-1);
>
> if (!opened) {
> +#ifndef WITHOUT_CAPSICUM
> + cap_rights_init(&rights, CAP_EVENT, CAP_IOCTL, CAP_READ, CAP_WRITE);
> + 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");
> +#endif
> ttyopen();
> opened = 1;
> }
Indentation looks wrong?
--
John Baldwin
_______________________________________________
[email protected] mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "[email protected]"
