On Wed, 2007-08-29 at 01:47 -0700, Chris Wright wrote:
> * George S. Coker, II (gscoker@xxxxxxxxxxxxxx) wrote:
> > - Updated for tip, 15730:256160ff19b7
> >
> > Signed-off-by: George Coker <gscoker@xxxxxxxxxxxxxx>
>
>
> > diff -r 256160ff19b7 -r bc357f5fe8cc Config.mk
> > --- a/Config.mk Thu Aug 16 13:27:59 2007 +0100
> > +++ b/Config.mk Wed Aug 22 16:26:21 2007 -0400
> > @@ -75,6 +75,10 @@ LDFLAGS += $(foreach i, $(EXTRA_LIB), -L
> > LDFLAGS += $(foreach i, $(EXTRA_LIB), -L$(i))
> > CFLAGS += $(foreach i, $(EXTRA_INCLUDES), -I$(i))
> >
> > +#Enable XSM security module. Enabling XSM requires selection of an
> > +#XSM security module.
> > +XSM_ENABLE ?= n
> > +
> > # If ACM_SECURITY = y, then the access control module is compiled
> > # into Xen and the policy type can be set by the boot policy file
> > # y - Build the Xen ACM framework
> > diff -r 256160ff19b7 -r bc357f5fe8cc xen/Makefile
> > --- a/xen/Makefile Thu Aug 16 13:27:59 2007 +0100
> > +++ b/xen/Makefile Wed Aug 22 16:26:21 2007 -0400
> > @@ -56,6 +56,7 @@ _clean: delete-unfresh-files
> > $(MAKE) -f $(BASEDIR)/Rules.mk -C common clean
> > $(MAKE) -f $(BASEDIR)/Rules.mk -C drivers clean
> > $(MAKE) -f $(BASEDIR)/Rules.mk -C acm clean
> > + $(MAKE) -f $(BASEDIR)/Rules.mk -C xsm clean
> > $(MAKE) -f $(BASEDIR)/Rules.mk -C arch/$(TARGET_ARCH) clean
> > rm -f include/asm *.o $(TARGET)* *~ core
> > rm -f include/asm-*/asm-offsets.h
> > @@ -122,7 +123,7 @@ build-headers:
> > build-headers:
> > $(MAKE) -C include/public/foreign
> >
> > -SUBDIRS = acm arch/$(TARGET_ARCH) common drivers
> > +SUBDIRS = xsm acm arch/$(TARGET_ARCH) common drivers
> > define all_sources
> > ( find include/asm-$(TARGET_ARCH) -name '*.h' -print; \
> > find include -name 'asm-*' -prune -o -name '*.h' -print; \
> > diff -r 256160ff19b7 -r bc357f5fe8cc xen/Rules.mk
> > --- a/xen/Rules.mk Thu Aug 16 13:27:59 2007 +0100
> > +++ b/xen/Rules.mk Wed Aug 22 16:26:21 2007 -0400
> > @@ -52,10 +52,12 @@ HDRS := $(filter-out %/asm-offsets.h,$(
> > # Note that link order matters!
> > ALL_OBJS-y += $(BASEDIR)/common/built_in.o
> > ALL_OBJS-y += $(BASEDIR)/drivers/built_in.o
> > +ALL_OBJS-y += $(BASEDIR)/xsm/built_in.o
> > ALL_OBJS-$(ACM_SECURITY) += $(BASEDIR)/acm/built_in.o
> > ALL_OBJS-y += $(BASEDIR)/arch/$(TARGET_ARCH)/built_in.o
> >
> > CFLAGS-y += -g -D__XEN__
> > +CFLAGS-$(XSM_ENABLE) += -DXSM_ENABLE
> > CFLAGS-$(ACM_SECURITY) += -DACM_SECURITY
> > CFLAGS-$(verbose) += -DVERBOSE
> > CFLAGS-$(crash_debug) += -DCRASH_DEBUG
> > diff -r 256160ff19b7 -r bc357f5fe8cc xen/arch/x86/domctl.c
> > --- a/xen/arch/x86/domctl.c Thu Aug 16 13:27:59 2007 +0100
> > +++ b/xen/arch/x86/domctl.c Wed Aug 22 16:26:21 2007 -0400
> > @@ -24,6 +24,7 @@
> > #include <asm/hvm/hvm.h>
> > #include <asm/hvm/support.h>
> > #include <asm/processor.h>
> > +#include <xsm/xsm.h>
> >
> > long arch_do_domctl(
> > struct xen_domctl *domctl,
> > @@ -41,6 +42,13 @@ long arch_do_domctl(
> > d = rcu_lock_domain_by_id(domctl->domain);
> > if ( d != NULL )
> > {
> > + ret = xsm_shadow_control(d, domctl->u.shadow_op.op);
> > + if ( ret )
> > + {
> > + rcu_unlock_domain(d);
> > + break;
> > + }
> > +
> > ret = paging_domctl(d,
> > &domctl->u.shadow_op,
> > guest_handle_cast(u_domctl, void));
>
> Any reason not to put this in paging_domctl, after some of its error
> checking?
>
Ack.
> > @@ -63,6 +71,14 @@ long arch_do_domctl(
> > ret = -ESRCH;
> > if ( unlikely((d = rcu_lock_domain_by_id(domctl->domain)) == NULL)
> > )
> > break;
> > +
> > + ret = xsm_ioport_permission(d, fp,
> > +
> > domctl->u.ioport_permission.allow_access);
> > + if ( ret )
> > + {
> > + rcu_unlock_domain(d);
> > + break;
> > + }
> >
> > if ( np == 0 )
> > ret = 0;
> > @@ -87,6 +103,13 @@ long arch_do_domctl(
> > if ( unlikely(!mfn_valid(mfn)) ||
> > unlikely((d = rcu_lock_domain_by_id(dom)) == NULL) )
> > break;
> > +
> > + ret = xsm_getpageframeinfo(mfn);
> > + if ( ret )
> > + {
> > + rcu_unlock_domain(d);
> > + break;
> > + }
> >
> > page = mfn_to_page(mfn);
>
> Should this check be here, and pass page?
Ack. I was trying to standardize the xsm interface on mfn's, but since
a module has to do the same kind of operation to resolve page owner for
checking, it is more efficient to pass page.
>
> > @@ -171,6 +194,10 @@ long arch_do_domctl(
> > struct page_info *page;
> > unsigned long mfn = arr32[j];
> >
> > + ret = xsm_getpageframeinfo(mfn);
> > + if ( ret )
> > + continue;
> > +
> > page = mfn_to_page(mfn);
>
> Same here.
>
Ack.
> > if ( likely(mfn_valid(mfn) && get_page(page, d)) )
>
> > diff -r 256160ff19b7 -r bc357f5fe8cc xen/arch/x86/hvm/hvm.c
> > --- a/xen/arch/x86/hvm/hvm.c Thu Aug 16 13:27:59 2007 +0100
> > +++ b/xen/arch/x86/hvm/hvm.c Wed Aug 22 16:26:21 2007 -0400
> > @@ -1,3 +1,4 @@
> > +
>
> Extra whitespace.
>
Ack.
> > /*
> > * hvm.c: Common hardware virtual machine abstractions.
> > *
> > @@ -1068,6 +1069,10 @@ static int hvmop_set_pci_intx_level(
> > if ( d == NULL )
> > return -ESRCH;
> >
> > + rc = xsm_hvm_set_pci_intx_level(d);
> > + if ( rc )
> > + goto out;
> > +
>
> Should IS_PRIV be handled here instead of a separate call?
> Also, I'd think it could be after is_hvm_domain (same with hvmcontext
> calls).
>
Ack. Moving after is_hvm_domain would be better. Folding IS_PRIV into
this check is not useful, b/c, ideally, IS_PRIV could be the
default/dummy behavior of this check.
> > rc = -EINVAL;
> > if ( !is_hvm_domain(d) )
> > goto out;
> > @@ -1111,6 +1116,10 @@ static int hvmop_set_isa_irq_level(
> > if ( d == NULL )
> > return -ESRCH;
> >
> > + rc = xsm_hvm_set_isa_irq_level(d);
> > + if ( rc )
> > + goto out;
> > +
>
> Same here
Ack.
>
> > rc = -EINVAL;
> > if ( !is_hvm_domain(d) )
> > goto out;
> > @@ -1153,6 +1162,10 @@ static int hvmop_set_pci_link_route(
> > d = rcu_lock_domain_by_id(op.domid);
> > if ( d == NULL )
> > return -ESRCH;
> > +
> > + rc = xsm_hvm_set_pci_link_route(d);
> > + if ( rc )
> > + goto out;
>
> and here
>
Ack.
> >
> > rc = -EINVAL;
> > if ( !is_hvm_domain(d) )
> > @@ -1196,6 +1209,10 @@ long do_hvm_op(unsigned long op, XEN_GUE
> >
> > if ( d == NULL )
> > return -ESRCH;
> > +
> > + rc = xsm_hvm_param(d, op);
> > + if ( rc )
> > + goto param_fail;
>
> and here (although IS_PRIV handling a bit different here)
>
Ack. Here the IS_PRIV check can't be simply relocated and maintain the
same semantics. If XSM became the default access control framework, the
semantics could be accomodated as follows,
the locking condition would be simplified to
if ( a.domid == DOMID_SELF )
d = rcu_lock_current_domain();
else
d = rcu_lock_domain_by_id(a.domid)
and the default/dummy hook behavior would emulate the displaced use of
IS_PRIV as,
if ( d->domid == DOMID_SELF )
return 0;
else if ( IS_PRIV(current->domain) )
return 0;
else
return -EPERM
> > --- a/xen/arch/x86/mm.c Thu Aug 16 13:27:59 2007 +0100
> > +++ b/xen/arch/x86/mm.c Wed Aug 22 16:26:21 2007 -0400
> > @@ -110,6 +110,7 @@
> > #include <asm/hypercall.h>
> > #include <asm/shared.h>
> > #include <public/memory.h>
> > +#include <xsm/xsm.h>
> >
> > #define MEM_LOG(_f, _a...) gdprintk(XENLOG_WARNING , _f "\n" , ## _a)
> >
> > @@ -2046,6 +2047,10 @@ int do_mmuext_op(
> > type = PGT_l4_page_table;
> >
> > pin_page:
> > + rc = xsm_memory_pin_page(current->domain, mfn);
> > + if ( rc )
> > + break;
> > +
>
> Why not pass page?
Ack.
>
> > /* Ignore pinning of invalid paging levels. */
> > if ( (op.cmd - MMUEXT_PIN_L1_TABLE) > (CONFIG_PAGING_LEVELS -
> > 1) )
> > break;
> > @@ -2331,6 +2336,10 @@ int do_mmu_update(
> > * MMU_NORMAL_PT_UPDATE: Normal update to any level of page
> > table.
> > */
> > case MMU_NORMAL_PT_UPDATE:
> > +
> > + rc = xsm_mmu_normal_update(current->domain, req.val);
> > + if ( rc )
> > + goto out;
>
> biglock is still held
Ack. better to break than goto here.
>
> >
> > gmfn = req.ptr >> PAGE_SHIFT;
> > mfn = gmfn_to_mfn(d, gmfn);
> > @@ -2422,6 +2431,10 @@ int do_mmu_update(
> > mfn = req.ptr >> PAGE_SHIFT;
> > gpfn = req.val;
> >
> > + rc = xsm_mmu_machphys_update(current->domain, mfn);
> > + if ( rc )
> > + goto out;
> > +
>
> same here
Ack. same.
>
> > if ( unlikely(!get_page_from_pagenr(mfn, FOREIGNDOM)) )
> > {
> > MEM_LOG("Could not get page for mach->phys update");
> > @@ -2800,6 +2813,10 @@ int do_update_va_mapping(unsigned long v
> > if ( unlikely(!__addr_ok(va) && !paging_mode_external(d)) )
> > return -EINVAL;
> >
> > + rc = xsm_update_va_mapping(current->domain, val);
> > + if ( rc )
> > + return rc;
> > +
> > LOCK_BIGLOCK(d);
> >
> > pl1e = guest_map_l1e(v, va, &gl1mfn);
> > @@ -3061,6 +3078,13 @@ long arch_memory_op(int op, XEN_GUEST_HA
> > else if ( (d = rcu_lock_domain_by_id(xatp.domid)) == NULL )
> > return -ESRCH;
> >
> > + if ( xsm_add_to_physmap(current->domain, d) )
> > + {
> > + if ( xatp.domid != DOMID_SELF )
> > + rcu_unlock_domain(d);
>
> rcu_unlock_domain shouldn't be conditional
>
Ack. clearly some detritus here, i think this was ok a few changesets
back.
> > + return -EPERM;
> > + }
> > +
> > switch ( xatp.space )
> > {
> > case XENMAPSPACE_shared_info:
> > @@ -3170,9 +3194,14 @@ long arch_memory_op(int op, XEN_GUEST_HA
> > struct xen_memory_map memmap;
> > XEN_GUEST_HANDLE(e820entry_t) buffer;
> > int count;
> > + int rc;
> >
> > if ( !IS_PRIV(current->domain) )
> > return -EINVAL;
> > +
> > + rc = xsm_machine_memory_map();
> > + if ( rc )
> > + return rc;
>
> Collapse IS_PRIV?
would like to, but some concern about too much change to xen's security
model in this patch round.
>
> > if ( copy_from_guest(&memmap, arg, 1) )
> > return -EFAULT;
>
> set_memory_map?
>
Ack. oops dropped hook, thanks
> > diff -r 256160ff19b7 -r bc357f5fe8cc xen/arch/x86/physdev.c
> > --- a/xen/arch/x86/physdev.c Thu Aug 16 13:27:59 2007 +0100
> > +++ b/xen/arch/x86/physdev.c Wed Aug 22 16:26:21 2007 -0400
> > @@ -12,6 +12,7 @@
> > #include <asm/hypercall.h>
> > #include <public/xen.h>
> > #include <public/physdev.h>
> > +#include <xsm/xsm.h>
> >
> > #ifndef COMPAT
> > typedef long ret_t;
> > @@ -73,6 +74,9 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_H
> > ret = -EPERM;
> > if ( !IS_PRIV(v->domain) )
> > break;
> > + ret = xsm_apic(current->domain, cmd);
> > + if ( ret )
> > + break;
>
> IS_PRIV, and use v instead of current
>
Ack on v. Is there a win to collapsing the check on ret and IS_PRIV
that I am missing or are you making a style point?
> > ret = ioapic_guest_read(apic.apic_physbase, apic.reg, &apic.value);
> > if ( copy_to_guest(arg, &apic, 1) != 0 )
> > ret = -EFAULT;
> > @@ -87,6 +91,9 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_H
> > ret = -EPERM;
> > if ( !IS_PRIV(v->domain) )
> > break;
> > + ret = xsm_apic(current->domain, cmd);
> > + if ( ret )
> > + break;
>
> IS_PRIV and use v instead of current
>
Ack on v.
> > ret = ioapic_guest_write(apic.apic_physbase, apic.reg, apic.value);
> > break;
> > }
> > @@ -100,6 +107,10 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_H
> >
> > ret = -EPERM;
> > if ( !IS_PRIV(v->domain) )
> > + break;
> > +
> > + ret = xsm_assign_vector(current->domain, irq_op.irq);
> > + if ( ret )
> > break;
>
> same here
Ack on v.
>
> >
> > irq = irq_op.irq;
> > diff -r 256160ff19b7 -r bc357f5fe8cc xen/arch/x86/platform_hypercall.c
> > --- a/xen/arch/x86/platform_hypercall.c Thu Aug 16 13:27:59 2007 +0100
> > +++ b/xen/arch/x86/platform_hypercall.c Wed Aug 22 16:26:21 2007 -0400
> > @@ -120,6 +137,11 @@ ret_t do_platform_op(XEN_GUEST_HANDLE(xe
> > case XENPF_microcode_update:
> > {
> > extern int microcode_update(XEN_GUEST_HANDLE(void), unsigned long
> > len);
> > +
> > + ret = xsm_microcode();
> > + if ( ret )
> > + break;
> > +
> > #ifndef COMPAT
> > ret = microcode_update(op->u.microcode.data,
> > op->u.microcode.length);
>
> Doesn't build on x86_64
>
I need to look into this. Why is not obvious to me here.
> > diff -r 256160ff19b7 -r bc357f5fe8cc xen/common/domain.c
> > --- a/xen/common/domain.c Thu Aug 16 13:27:59 2007 +0100
> > +++ b/xen/common/domain.c Wed Aug 22 16:26:21 2007 -0400
> > @@ -29,6 +29,7 @@
> > #include <public/sched.h>
> > #include <public/vcpu.h>
> > #include <acm/acm_hooks.h>
> > +#include <xsm/xsm.h>
> >
> > /* Protect updates/reads (resp.) of domain_list and domain_hash. */
> > DEFINE_SPINLOCK(domlist_update_lock);
> > @@ -57,6 +58,13 @@ struct domain *alloc_domain(domid_t domi
> >
> > memset(d, 0, sizeof(*d));
> > d->domain_id = domid;
> > +
> > + if ( xsm_alloc_security_domain(d) != 0 )
> > + {
> > + free_domain(d);
>
> xfree?
handled in free_domain.
>
> > + return NULL;
> > + }
> > +
> > atomic_set(&d->refcnt, 1);
> > spin_lock_init(&d->big_lock);
> > spin_lock_init(&d->page_alloc_lock);
> > @@ -69,6 +77,7 @@ struct domain *alloc_domain(domid_t domi
> >
> > void free_domain(struct domain *d)
> > {
> > + xsm_free_security_domain(d);
> > xfree(d);
> > }
> >
> > @@ -193,6 +202,9 @@ struct domain *domain_create(
> >
> > if ( !is_idle_domain(d) )
> > {
> > + if ( xsm_domain_create(d, ssidref) != 0 )
> > + goto fail;
> > +
> > d->is_paused_by_controller = 1;
> > atomic_inc(&d->pause_count);
> >
> > diff -r 256160ff19b7 -r bc357f5fe8cc xen/common/domctl.c
> > --- a/xen/common/domctl.c Thu Aug 16 13:27:59 2007 +0100
> > +++ b/xen/common/domctl.c Wed Aug 22 16:26:21 2007 -0400
> > @@ -24,6 +24,7 @@
> > #include <asm/current.h>
> > #include <public/domctl.h>
> > #include <acm/acm_hooks.h>
> > +#include <xsm/xsm.h>
> >
> > extern long arch_do_domctl(
> > struct xen_domctl *op, XEN_GUEST_HANDLE(xen_domctl_t) u_domctl);
> > @@ -127,7 +128,9 @@ void getdomaininfo(struct domain *d, str
> > info->ssidref = ((struct acm_ssid_domain *)d->ssid)->ssidref;
> > else
> > info->ssidref = ACM_DEFAULT_SSID;
> > -
> > +
> > + xsm_security_domaininfo(d, info);
> > +
> > info->tot_pages = d->tot_pages;
> > info->max_pages = d->max_pages;
> > info->shared_info_frame = mfn_to_gmfn(d,
> > __pa(d->shared_info)>>PAGE_SHIFT);
> > @@ -204,6 +207,13 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
> > if ( d == NULL )
> > break;
> >
> > + ret = xsm_setvcpucontext(d);
> > + if ( ret )
> > + {
> > + rcu_unlock_domain(d);
> > + break;
>
> just use goto svc_out like others?
>
Ack. xfree(c.nat) when c.nat is NULL seems ok.
> > + }
> > +
> > ret = -EINVAL;
> > if ( (vcpu >= MAX_VIRT_CPUS) || ((v = d->vcpu[vcpu]) == NULL) )
> > goto svc_out;
> > @@ -251,12 +261,17 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
> > ret = -ESRCH;
> > if ( d != NULL )
> > {
> > + ret = xsm_pausedomain(d);
> > + if ( ret )
> > + goto pausedomain_out;
> > +
> > ret = -EINVAL;
> > if ( d != current->domain )
> > {
> > domain_pause_by_systemcontroller(d);
> > ret = 0;
> > }
> > + pausedomain_out:
> > rcu_unlock_domain(d);
> > }
> > }
> > @@ -270,7 +285,13 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
> > if ( d == NULL )
> > break;
> >
> > + ret = xsm_unpausedomain(d);
> > + if ( ret )
> > + goto unpausedomain_out;
> > +
> > domain_unpause_by_systemcontroller(d);
> > +
> > + unpausedomain_out:
> > rcu_unlock_domain(d);
> > ret = 0;
>
> lost error return value
Ack.
>
> > }
> > @@ -283,6 +304,13 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
> > ret = -ESRCH;
> > if ( d == NULL )
> > break;
> > +
> > + ret = xsm_resumedomain(d);
> > + if ( ret )
> > + {
> > + rcu_unlock_domain(d);
> > + break;
> > + }
> >
> > domain_resume(d);
> > rcu_unlock_domain(d);
> > @@ -359,6 +387,13 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
> > if ( (d = rcu_lock_domain_by_id(op->domain)) == NULL )
> > break;
> >
> > + ret = xsm_max_vcpus(d);
> > + if ( ret )
> > + {
> > + rcu_unlock_domain(d);
> > + break;
> > + }
> > +
> > /* Needed, for example, to ensure writable p.t. state is synced. */
> > domain_pause(d);
> >
> > @@ -395,12 +430,18 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
> > ret = -ESRCH;
> > if ( d != NULL )
> > {
> > + ret = xsm_destroydomain(d);
> > + if ( ret )
> > + goto destroydomain_out;
> > +
> > ret = -EINVAL;
> > if ( d != current->domain )
> > {
> > domain_kill(d);
> > ret = 0;
> > }
> > +
> > + destroydomain_out:
> > rcu_unlock_domain(d);
> > }
> > }
> > @@ -418,6 +459,10 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
> > if ( d == NULL )
> > break;
> >
> > + ret = xsm_vcpuaffinity(op->cmd, d);
> > + if ( ret )
> > + goto vcpuaffinity_out;
> > +
> > ret = -EINVAL;
> > if ( op->u.vcpuaffinity.vcpu >= MAX_VIRT_CPUS )
> > goto vcpuaffinity_out;
> > @@ -452,10 +497,15 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
> > if ( (d = rcu_lock_domain_by_id(op->domain)) == NULL )
> > break;
> >
> > + ret = xsm_scheduler(d);
> > + if ( ret )
> > + goto scheduler_op_out;
> > +
> > ret = sched_adjust(d, &op->u.scheduler_op);
> > if ( copy_to_guest(u_domctl, op, 1) )
> > ret = -EFAULT;
> >
> > + scheduler_op_out:
> > rcu_unlock_domain(d);
> > }
> > break;
> > @@ -478,12 +528,17 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
> > break;
> > }
> >
> > + ret = xsm_getdomaininfo(d);
> > + if ( ret )
> > + goto getdomaininfo_out;
> > +
>
> can use this new goto in d == NULL cleanup
>
It's not clear which style is preferred, local cleanups or goto's that
ensure cleanup is done consistently.
> > getdomaininfo(d, &op->u.getdomaininfo);
> >
> > op->domain = op->u.getdomaininfo.domain;
> > if ( copy_to_guest(u_domctl, op, 1) )
> > ret = -EFAULT;
> >
> > + getdomaininfo_out:
> > rcu_read_unlock(&domlist_read_lock);
> > }
> > break;
> > @@ -497,6 +552,13 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
> > ret = -ESRCH;
> > if ( (d = rcu_lock_domain_by_id(op->domain)) == NULL )
> > break;
> > +
> > + ret = xsm_getvcpucontext(d);
> > + if ( ret )
> > + {
> > + rcu_unlock_domain(d);
> > + break;
> > + }
>
> can goto getvcpucontext_out
Ack.
>
> > ret = -EINVAL;
> > if ( op->u.vcpucontext.vcpu >= MAX_VIRT_CPUS )
> > diff -r 256160ff19b7 -r bc357f5fe8cc xen/common/kexec.c
> > --- a/xen/common/kexec.c Thu Aug 16 13:27:59 2007 +0100
> > +++ b/xen/common/kexec.c Wed Aug 22 16:26:21 2007 -0400
> > @@ -21,6 +21,7 @@
> > #include <xen/version.h>
> > #include <xen/console.h>
> > #include <public/elfnote.h>
> > +#include <xsm/xsm.h>
> >
> > #ifndef COMPAT
> >
> > @@ -367,6 +368,10 @@ ret_t do_kexec_op(unsigned long op, XEN_
> > if ( !IS_PRIV(current->domain) )
> > return -EPERM;
> >
> > + ret = xsm_kexec();
> > + if ( ret )
> > + return ret;
> > +
>
> IS_PRIV collapse?
Ack, same as other IS_PRIV ?'s.
>
> > switch ( op )
> > {
> > case KEXEC_CMD_kexec_get_range:
> > diff -r 256160ff19b7 -r bc357f5fe8cc xen/common/memory.c
> > --- a/xen/common/memory.c Thu Aug 16 13:27:59 2007 +0100
> > +++ b/xen/common/memory.c Wed Aug 22 16:26:21 2007 -0400
> > @@ -22,6 +22,7 @@
> > #include <asm/current.h>
> > #include <asm/hardirq.h>
> > #include <public/memory.h>
> > +#include <xsm/xsm.h>
> >
> > struct memop_args {
> > /* INPUT */
> > @@ -216,6 +217,7 @@ static long translate_gpfn_list(
> > xen_pfn_t gpfn;
> > xen_pfn_t mfn;
> > struct domain *d;
> > + int rc;
> >
> > if ( copy_from_guest(&op, uop, 1) )
> > return -EFAULT;
> > @@ -258,6 +260,13 @@ static long translate_gpfn_list(
> > }
> >
> > mfn = gmfn_to_mfn(d, gpfn);
> > +
> > + rc = xsm_translate_gpfn_list(current->domain, mfn);
> > + if ( rc )
> > + {
> > + rcu_unlock_domain(d);
> > + return rc;
> > + }
>
> IS_PRIV?
>
checked earlier, this check is deeper in the code
> > if ( unlikely(__copy_to_guest_offset(op.mfn_list, i, &mfn, 1)) )
> > {
> > @@ -538,6 +547,14 @@ long do_memory_op(unsigned long cmd, XEN
> > return start_extent;
> > args.domain = d;
> >
> > + rc = xsm_memory_adjust_reservation(current->domain, d);
> > + if ( rc )
> > + {
> > + if ( reservation.domid != DOMID_SELF )
> > + rcu_unlock_domain(d);
> > + return rc;
> > + }
> > +
> > switch ( op )
> > {
> > case XENMEM_increase_reservation:
> > @@ -583,6 +600,14 @@ long do_memory_op(unsigned long cmd, XEN
> > return -EPERM;
> > else if ( (d = rcu_lock_domain_by_id(domid)) == NULL )
> > return -ESRCH;
> > +
> > + rc = xsm_memory_stat_reservation(current->domain, d);
> > + if ( rc )
> > + {
> > + if ( domid != DOMID_SELF )
> > + rcu_unlock_domain(d);
> > + return rc;
> > + }
> >
> > switch ( op )
> > {
> > diff -r 256160ff19b7 -r bc357f5fe8cc xen/common/schedule.c
> > --- a/xen/common/schedule.c Thu Aug 16 13:27:59 2007 +0100
> > +++ b/xen/common/schedule.c Wed Aug 22 16:26:21 2007 -0400
> > @@ -32,6 +32,7 @@
> > #include <xen/guest_access.h>
> > #include <xen/multicall.h>
> > #include <public/sched.h>
> > +#include <xsm/xsm.h>
> >
> > /* opt_sched: scheduler - default to credit */
> > static char opt_sched[10] = "credit";
> > @@ -460,6 +461,13 @@ ret_t do_sched_op(int cmd, XEN_GUEST_HAN
> > d = rcu_lock_domain_by_id(sched_remote_shutdown.domain_id);
> > if ( d == NULL )
> > break;
> > +
> > + ret = xsm_schedop_shutdown(current->domain, d);
> > + if ( ret )
> > + {
> > + rcu_unlock_domain(d);
> > + return ret;
> > + }
> >
> > /* domain_pause() prevens any further execution in guest context.
> > */
> > domain_pause(d);
> > diff -r 256160ff19b7 -r bc357f5fe8cc xen/common/sysctl.c
> > --- a/xen/common/sysctl.c Thu Aug 16 13:27:59 2007 +0100
> > +++ b/xen/common/sysctl.c Wed Aug 22 16:26:21 2007 -0400
> > @@ -23,6 +23,7 @@
> > #include <public/sysctl.h>
> > #include <asm/numa.h>
> > #include <xen/nodemask.h>
> > +#include <xsm/xsm.h>
> >
> > extern long arch_do_sysctl(
> > struct xen_sysctl *op, XEN_GUEST_HANDLE(xen_sysctl_t) u_sysctl);
> > @@ -48,6 +49,10 @@ long do_sysctl(XEN_GUEST_HANDLE(xen_sysc
> > {
> > case XEN_SYSCTL_readconsole:
> > {
> > + ret = xsm_readconsole(op->u.readconsole.clear);
> > + if ( ret )
> > + break;
> > +
> > ret = read_console_ring(
> > guest_handle_cast(op->u.readconsole.buffer, char),
> > &op->u.readconsole.count,
> > @@ -59,6 +64,10 @@ long do_sysctl(XEN_GUEST_HANDLE(xen_sysc
> >
> > case XEN_SYSCTL_tbuf_op:
> > {
> > + ret = xsm_tbufcontrol();
> > + if ( ret )
> > + break;
> > +
> > ret = tb_control(&op->u.tbuf_op);
> > if ( copy_to_guest(u_sysctl, op, 1) )
> > ret = -EFAULT;
> > @@ -67,6 +76,10 @@ long do_sysctl(XEN_GUEST_HANDLE(xen_sysc
> >
> > case XEN_SYSCTL_sched_id:
> > {
> > + ret = xsm_sched_id();
> > + if ( ret )
> > + break;
> > +
> > op->u.sched_id.sched_id = sched_id();
> > if ( copy_to_guest(u_sysctl, op, 1) )
> > ret = -EFAULT;
> > @@ -90,6 +103,10 @@ long do_sysctl(XEN_GUEST_HANDLE(xen_sysc
> > if ( num_domains == op->u.getdomaininfolist.max_domains )
> > break;
> >
> > + ret = xsm_getdomaininfo(d);
> > + if ( ret )
> > + continue;
> > +
> > getdomaininfo(d, &info);
> >
> > if ( copy_to_guest_offset(op->u.getdomaininfolist.buffer,
> > @@ -117,6 +134,10 @@ long do_sysctl(XEN_GUEST_HANDLE(xen_sysc
> > #ifdef PERF_COUNTERS
> > case XEN_SYSCTL_perfc_op:
> > {
> > + ret = xsm_perfcontrol();
> > + if ( ret )
> > + break;
> > +
> > ret = perfc_control(&op->u.perfc_op);
> > if ( copy_to_guest(u_sysctl, op, 1) )
> > ret = -EFAULT;
> > diff -r 256160ff19b7 -r bc357f5fe8cc xen/common/xenoprof.c
> > --- a/xen/common/xenoprof.c Thu Aug 16 13:27:59 2007 +0100
> > +++ b/xen/common/xenoprof.c Wed Aug 22 16:26:21 2007 -0400
> > @@ -14,6 +14,7 @@
> > #include <xen/sched.h>
> > #include <public/xenoprof.h>
> > #include <xen/paging.h>
> > +#include <xsm/xsm.h>
> >
> > /* Limit amount of pages used for shared buffer (per domain) */
> > #define MAX_OPROF_SHARED_PAGES 32
> > @@ -634,6 +635,10 @@ int do_xenoprof_op(int op, XEN_GUEST_HAN
> > return -EPERM;
> > }
> >
> > + ret = xsm_profile(current->domain, op);
> > + if ( ret )
> > + return ret;
> > +
> > spin_lock(&xenoprof_lock);
> >
> > switch ( op )
> > diff -r 256160ff19b7 -r bc357f5fe8cc xen/drivers/char/console.c
> > --- a/xen/drivers/char/console.c Thu Aug 16 13:27:59 2007 +0100
> > +++ b/xen/drivers/char/console.c Wed Aug 22 16:26:21 2007 -0400
> > @@ -32,6 +32,7 @@
> > #include <asm/debugger.h>
> > #include <asm/io.h>
> > #include <asm/div64.h>
> > +#include <xsm/xsm.h>
> >
> > /* console: comma-separated list of console outputs. */
> > static char opt_console[30] = OPT_CONSOLE_STR;
> > @@ -357,6 +358,10 @@ long do_console_io(int cmd, int count, X
> > if ( current->domain->domain_id != 0 )
> > return -EPERM;
> > #endif
> > +
> > + rc = xsm_console_io(current->domain, cmd);
> > + if ( rc )
> > + return rc;
> >
> > switch ( cmd )
> > {
> > --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> > +++ b/xen/include/xsm/xsm.h Wed Aug 22 16:26:21 2007 -0400
> > @@ -0,0 +1,537 @@
> > +/*
> > + * This file contains the XSM hook definitions for Xen.
> > + *
> > + * This work is based on the LSM implementation in Linux 2.6.13.4.
> > + *
> > + * Author: George Coker, <gscoker@xxxxxxxxxxxxxx>
> > + *
> > + * Contributors: Michael LeMay, <mdlemay@xxxxxxxxxxxxxx>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2,
> > + * as published by the Free Software Foundation.
> > + */
> > +
> > +#ifndef __XSM_H__
> > +#define __XSM_H__
> > +
> > +#include <xen/sched.h>
> > +#include <xen/multiboot.h>
> > +
> > +typedef void xsm_op_t;
> > +DEFINE_XEN_GUEST_HANDLE(xsm_op_t);
> > +
> > +extern long do_xsm_op (XEN_GUEST_HANDLE(xsm_op_t) op);
> > +
> > +#ifdef XSM_ENABLE
> > + #define xsm_call(fn) xsm_ops->fn
> > +#else
> > + #define xsm_call(fn) 0
> > +#endif
> > +
> > +/* policy magic number (defined by XSM_MAGIC) */
> > +typedef u32 xsm_magic_t;
> > +#ifndef XSM_MAGIC
> > +#define XSM_MAGIC 0x00000000
> > +#endif
> > +
> > +#ifdef XSM_ENABLE
> > +
> > +extern char *policy_buffer;
> > +extern u32 policy_size;
> > +
> > +typedef int (*xsm_initcall_t)(void);
> > +
> > +extern xsm_initcall_t __xsm_initcall_start[], __xsm_initcall_end[];
> > +
> > +#define xsm_initcall(fn) \
> > + static xsm_initcall_t __initcall_##fn \
> > + __attribute_used__ __attribute__((__section__(".xsm_initcall.init")))
> > = fn
> > +
>
> This could benefit from some comments re: usage for each hook.
>
Ack. working on it.
> > +struct xsm_operations {
> > + void (*security_domaininfo) (struct domain *d,
> > + struct xen_domctl_getdomaininfo
> > *info);
> > + int (*setvcpucontext) (struct domain *d);
> > + int (*pausedomain) (struct domain *d);
> > + int (*unpausedomain) (struct domain *d);
> > + int (*resumedomain) (struct domain *d);
> > + int (*domain_create) (struct domain *d, u32 ssidref);
> > + int (*max_vcpus) (struct domain *d);
> > + int (*destroydomain) (struct domain *d);
> > + int (*vcpuaffinity) (int cmd, struct domain *d);
> > + int (*scheduler) (struct domain *d);
> > + int (*getdomaininfo) (struct domain *d);
> > + int (*getvcpucontext) (struct domain *d);
> > + int (*getvcpuinfo) (struct domain *d);
> > + int (*domain_settime) (struct domain *d);
> > + int (*tbufcontrol) (void);
> > + int (*readconsole) (uint32_t clear);
> > + int (*sched_id) (void);
> > + int (*setdomainmaxmem) (struct domain *d);
> > + int (*setdomainhandle) (struct domain *d);
> > + int (*setdebugging) (struct domain *d);
> > + int (*irq_permission) (struct domain *d, uint8_t pirq, uint8_t access);
> > + int (*iomem_permission) (struct domain *d, unsigned long mfn,
> > + uint8_t
> > access);
> > + int (*perfcontrol) (void);
> > +
> > + int (*evtchn_unbound) (struct domain *d, struct evtchn *chn, domid_t
> > id2);
> > + int (*evtchn_interdomain) (struct domain *d1, struct evtchn *chn1,
> > + struct domain *d2, struct evtchn
> > *chn2);
> > + void (*evtchn_close_post) (struct evtchn *chn);
> > + int (*evtchn_send) (struct domain *d, struct evtchn *chn);
> > + int (*evtchn_status) (struct domain *d, struct evtchn *chn);
> > + int (*evtchn_reset) (struct domain *d1, struct domain *d2);
> > +
> > + int (*grant_mapref) (struct domain *d1, struct domain *d2, uint32_t
> > flags);
> > + int (*grant_unmapref) (struct domain *d1, struct domain *d2);
> > + int (*grant_setup) (struct domain *d1, struct domain *d2);
> > + int (*grant_transfer) (struct domain *d1, struct domain *d2);
> > + int (*grant_copy) (struct domain *d1, struct domain *d2);
> > + int (*grant_query_size) (struct domain *d1, struct domain *d2);
> > +
> > + int (*alloc_security_domain) (struct domain *d);
> > + void (*free_security_domain) (struct domain *d);
> > + int (*alloc_security_evtchn) (struct evtchn *chn);
> > + void (*free_security_evtchn) (struct evtchn *chn);
> > +
> > + int (*translate_gpfn_list) (struct domain *d, unsigned long mfn);
> > + int (*memory_adjust_reservation) (struct domain *d1, struct domain
> > *d2);
> > + int (*memory_stat_reservation) (struct domain *d1, struct domain *d2);
> > + int (*memory_pin_page) (struct domain *d, unsigned long mfn);
> > + int (*update_va_mapping) (struct domain *d, l1_pgentry_t pte);
> > +
> > + int (*console_io) (struct domain *d, int cmd);
> > +
> > + int (*profile) (struct domain *d, int op);
> > +
> > + int (*kexec) (void);
> > + int (*schedop_shutdown) (struct domain *d1, struct domain *d2);
> > +
> > + long (*__do_xsm_op) (XEN_GUEST_HANDLE(xsm_op_t) op);
> > + void (*complete_init) (struct domain *d);
> > +
> > +#ifdef CONFIG_X86
> > + int (*shadow_control) (struct domain *d, uint32_t op);
> > + int (*ioport_permission) (struct domain *d, uint32_t ioport,
> > + uint8_t
> > access);
> > + int (*getpageframeinfo) (unsigned long mfn);
> > + int (*getmemlist) (struct domain *d);
> > + int (*hypercall_init) (struct domain *d);
> > + int (*hvmcontext) (struct domain *d, uint32_t op);
> > + int (*address_size) (struct domain *d, uint32_t op);
> > + int (*hvm_param) (struct domain *d, unsigned long op);
> > + int (*hvm_set_pci_intx_level) (struct domain *d);
> > + int (*hvm_set_isa_irq_level) (struct domain *d);
> > + int (*hvm_set_pci_link_route) (struct domain *d);
> > + int (*apic) (struct domain *d, int cmd);
> > + int (*assign_vector) (struct domain *d, uint32_t pirq);
> > + int (*xen_settime) (void);
> > + int (*memtype) (uint32_t access);
> > + int (*microcode) (void);
> > + int (*physinfo) (void);
> > + int (*platform_quirk) (uint32_t);
> > + int (*machine_memory_map) (void);
> > + int (*domain_memory_map) (struct domain *d1, struct domain *d);
> > + int (*mmu_normal_update) (struct domain *d, intpte_t fpte);
> > + int (*mmu_machphys_update) (struct domain *d, unsigned long mfn);
> > + int (*add_to_physmap) (struct domain *d1, struct domain *d2);
> > +#endif
> > +};
> > +
> > +#endif
> > +
> > +extern struct xsm_operations *xsm_ops;
> > +++ b/xen/xsm/dummy.c Wed Aug 22 16:26:21 2007 -0400
> > @@ -0,0 +1,490 @@
> > +/*
> > + * This file contains the Flask hook function implementations for Xen.
>
> These aren't the flask hooks.
>
Ack.
> > --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> > +++ b/xen/xsm/xsm_core.c Wed Aug 22 16:26:21 2007 -0400
> > @@ -0,0 +1,120 @@
> > +/*
> > + * This file contains the Flask hook function implementations for Xen.
>
> neither is this
>
Ack.
> > + * This work is based on the LSM implementation in Linux 2.6.13.4.
> > + *
> > + * Author: George Coker, <gscoker@xxxxxxxxxxxxxx>
> > + *
> > + * Contributors: Michael LeMay, <mdlemay@xxxxxxxxxxxxxx>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2,
> > + * as published by the Free Software Foundation.
> > + */
> > +
> > +#include <xen/init.h>
> > +#include <xen/errno.h>
> > +#include <xen/lib.h>
> > +
> > +#include <xsm/xsm.h>
> > +
> > +#ifdef XSM_ENABLE
> > +
> > +#define XSM_FRAMEWORK_VERSION "1.0.0"
>
> Could probably go, never really gets used
>
Just seems like a tidy way to bump version number without scrutinize
code that probably won't change much over the life of XSM.
> > +
> > +extern struct xsm_operations dummy_xsm_ops;
> > +extern void xsm_fixup_ops(struct xsm_operations *ops);
> > +
> > +struct xsm_operations *xsm_ops;
> > +
> > +static inline int verify(struct xsm_operations *ops)
> > +{
> > + /* verify the security_operations structure exists */
> > + if ( !ops )
> > + return -EINVAL;
> > + xsm_fixup_ops(ops);
> > + return 0;
> > +}
> > +
> > +static void __init do_xsm_initcalls(void)
> > +{
> > + xsm_initcall_t *call;
> > + call = __xsm_initcall_start;
> > + while ( call < __xsm_initcall_end )
> > + {
> > + (*call) ();
> > + call++;
> > + }
> > +}
> > +
> > +int __init xsm_init(unsigned int *initrdidx, const multiboot_info_t *mbi,
> > + unsigned long initial_images_start)
> > +{
> > + int ret = 0;
> > +
> > + printk("XSM Framework v" XSM_FRAMEWORK_VERSION " initialized\n");
> > +
> > + if ( XSM_MAGIC )
>
> Seems as if it's nearly compiled in, wonder if hook init could be made
> compile time too?
I'm not sure, but I don't think so. b/c
- a desire to keep XSM module neutral
- minimize the impact of XSM requirements of one module on other modules
Either way, there might be some simplification that can be done to the
init and registration.
> > + {
> > + ret = xsm_policy_init(initrdidx, mbi, initial_images_start);
> > + if ( ret )
> > + {
> > + printk("%s: Error initializing policy.\n", __FUNCTION__);
> > + return -EINVAL;
> > + }
> > + }
> > +
> > + if ( verify(&dummy_xsm_ops) )
> > + {
> > + printk("%s could not verify "
> > + "dummy_xsm_ops structure.\n", __FUNCTION__);
> > + return -EIO;
> > + }
> > +
> > + xsm_ops = &dummy_xsm_ops;
> > + do_xsm_initcalls();
> > +
> > + return 0;
> > +}
> > +
> > +int register_xsm(struct xsm_operations *ops)
> > +{
> > + if ( verify(ops) )
> > + {
> > + printk("%s could not verify "
> > + "security_operations structure.\n", __FUNCTION__);
> > + return -EINVAL;
> > + }
> > +
> > + if ( xsm_ops != &dummy_xsm_ops )
> > + return -EAGAIN;
> > +
> > + xsm_ops = ops;
> > +
> > + return 0;
> > +}
> > +
> > +
> > +int unregister_xsm(struct xsm_operations *ops)
> > +{
> > + if ( ops != xsm_ops )
> > + {
> > + printk("%s: trying to unregister "
> > + "a security_opts structure that is not "
> > + "registered, failing.\n", __FUNCTION__);
> > + return -EINVAL;
> > + }
> > +
> > + xsm_ops = &dummy_xsm_ops;
> > +
> > + return 0;
> > +}
> > +
> > +#endif
> > +
> > +long do_xsm_op (XEN_GUEST_HANDLE(xsm_op_t) op)
> > +{
> > + return __do_xsm_op(op);
> > +}
> > +
> > +
_______________________________________________
Xense-devel mailing list
Xense-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xense-devel
|
Home