Re: [PATCH] [Xen] [ACM] Code restructuring

Re: [PATCH] [Xen] [ACM] Code restructuring

[Keir Fraser]

Much nicer, thank you. I've applied it for 3.0.5. However, given the effort
put in to labelling a domain at the very start of its lifetime, wouldn't it
be a good idea to leave that label attached until the domain is actually
destroyed? This would correspond to putting the destructor call in
domain_destroy() or, even better, complete_domain_destroy(). Bear in mind
that a domain may still be accessible and have resources allocated to it
even after domain_kill() finishes executing. So stripping its resource label
at that point in time is a bit worrying.

 -- Keir

On 24/4/07 17:17, "Stefan Berger" <stefanb@us.ibm.com> wrote:

> This patch restructures part of the ACM code in Xen. When a domain is
> created, the function acm_domain_create() in domain_create() is called
> that does what previously the pre- and post_domain_create functions were
> doing. Similarly there's a function acm_domain_destroy() in
> domain_kill() that reverts changes to state when destroying a domain.
> There's no more separate initialization necessary for domain-0, but
> domain_create takes one more parameter, the ssidref. It is usually
> passed through the hypercall when a domain is created.
> 
> Signed-off-by: Stefan Berger <stefanb@us.ibm.com>

Re: [PATCH] [Xen] [ACM] Code restructuring

[Stefan Berger]

[-- Attachment #1.1: Type: text/plain, Size: 1435 bytes --]

Keir Fraser <keir@xensource.com> wrote on 04/24/2007 11:55:51 AM:

> Much nicer, thank you. I've applied it for 3.0.5. However, given the 
effort
> put in to labelling a domain at the very start of its lifetime, wouldn't 
it
> be a good idea to leave that label attached until the domain is actually
> destroyed? This would correspond to putting the destructor call in
> domain_destroy() or, even better, complete_domain_destroy(). Bear in 
mind
> that a domain may still be accessible and have resources allocated to it
> even after domain_kill() finishes executing. So stripping its resource 
label
> at that point in time is a bit worrying.

Right. Can you move this one call into complete_domain_destroy()?

   Stefan
> 
>  -- Keir
> 
> On 24/4/07 17:17, "Stefan Berger" <stefanb@us.ibm.com> wrote:
> 
> > This patch restructures part of the ACM code in Xen. When a domain is
> > created, the function acm_domain_create() in domain_create() is called
> > that does what previously the pre- and post_domain_create functions 
were
> > doing. Similarly there's a function acm_domain_destroy() in
> > domain_kill() that reverts changes to state when destroying a domain.
> > There's no more separate initialization necessary for domain-0, but
> > domain_create takes one more parameter, the ssidref. It is usually
> > passed through the hypercall when a domain is created.
> > 
> > Signed-off-by: Stefan Berger <stefanb@us.ibm.com>
> 

[-- Attachment #1.2: Type: text/html, Size: 1787 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

[PATCH] [Xen] [ACM] Code restructuring

[Stefan Berger]

[-- Attachment #1: Type: text/plain, Size: 586 bytes --]

This patch restructures part of the ACM code in Xen. When a domain is
created, the function acm_domain_create() in domain_create() is called
that does what previously the pre- and post_domain_create functions were
doing. Similarly there's a function acm_domain_destroy() in
domain_kill() that reverts changes to state when destroying a domain.
There's no more separate initialization necessary for domain-0, but
domain_create takes one more parameter, the ssidref. It is usually
passed through the hypercall when a domain is created.

Signed-off-by: Stefan Berger <stefanb@us.ibm.com>


[-- Attachment #2: xen_acm_restructure.diff --]
[-- Type: text/x-patch, Size: 25441 bytes --]

---
 xen-unstable.hg/xen/acm/acm_chinesewall_hooks.c             |   70 +---
 xen-unstable.hg/xen/acm/acm_null_hooks.c                    |    6 
 xen-unstable.hg/xen/acm/acm_simple_type_enforcement_hooks.c |   17 -
 xen-unstable.hg/xen/arch/ia64/xen/xensetup.c                |    4 
 xen-unstable.hg/xen/arch/powerpc/setup.c                    |    7 
 xen-unstable.hg/xen/arch/x86/setup.c                        |    8 
 xen-unstable.hg/xen/common/domain.c                         |   22 +
 xen-unstable.hg/xen/common/domctl.c                         |   12 
 xen-unstable.hg/xen/include/acm/acm_hooks.h                 |  190 +++---------
 xen-unstable.hg/xen/include/xen/sched.h                     |    4 
 10 files changed, 119 insertions(+), 221 deletions(-)

Index: root/xen-unstable.hg/xen/common/domctl.c
===================================================================
--- root.orig/xen-unstable.hg/xen/common/domctl.c
+++ root/xen-unstable.hg/xen/common/domctl.c
@@ -176,7 +176,6 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
 {
     long ret = 0;
     struct xen_domctl curop, *op = &curop;
-    void *ssid = NULL; /* save security ptr between pre and post/fail hooks */
     static DEFINE_SPINLOCK(domctl_lock);
 
     if ( !IS_PRIV(current->domain) )
@@ -188,9 +187,6 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
     if ( op->interface_version != XEN_DOMCTL_INTERFACE_VERSION )
         return -EACCES;
 
-    if ( acm_pre_domctl(op, &ssid) )
-        return -EPERM;
-
     spin_lock(&domctl_lock);
 
     switch ( op->cmd )
@@ -333,7 +329,8 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
             domcr_flags |= DOMCRF_hvm;
 
         ret = -ENOMEM;
-        if ( (d = domain_create(dom, domcr_flags)) == NULL )
+        if ( (d = domain_create(dom, domcr_flags,
+                                op->u.createdomain.ssidref)) == NULL )
             break;
 
         ret = 0;
@@ -716,11 +713,6 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
 
     spin_unlock(&domctl_lock);
 
-    if ( ret == 0 )
-        acm_post_domctl(op, &ssid);
-    else
-        acm_fail_domctl(op, &ssid);
-
     return ret;
 }
 
Index: root/xen-unstable.hg/xen/arch/x86/setup.c
===================================================================
--- root.orig/xen-unstable.hg/xen/arch/x86/setup.c
+++ root/xen-unstable.hg/xen/arch/x86/setup.c
@@ -254,7 +254,7 @@ static void __init init_idle_domain(void
     /* Domain creation requires that scheduler structures are initialised. */
     scheduler_init();
 
-    idle_domain = domain_create(IDLE_DOMAIN_ID, 0);
+    idle_domain = domain_create(IDLE_DOMAIN_ID, 0, 0);
     if ( (idle_domain == NULL) || (alloc_vcpu(idle_domain, 0, 0) == NULL) )
         BUG();
 
@@ -727,15 +727,13 @@ void __init __start_xen(multiboot_info_t
     acm_init(_policy_start, _policy_len);
 
     /* Create initial domain 0. */
-    dom0 = domain_create(0, 0);
+    dom0 = domain_create(0, 0, DOM0_SSIDREF);
+
     if ( (dom0 == NULL) || (alloc_vcpu(dom0, 0, 0) == NULL) )
         panic("Error creating domain 0\n");
 
     dom0->is_privileged = 1;
 
-    /* Post-create hook sets security label. */
-    acm_post_domain0_create(dom0->domain_id);
-
     /* Grab the DOM0 command line. */
     cmdline = (char *)(mod[0].string ? __va(mod[0].string) : NULL);
     if ( cmdline != NULL )
Index: root/xen-unstable.hg/xen/common/domain.c
===================================================================
--- root.orig/xen-unstable.hg/xen/common/domain.c
+++ root/xen-unstable.hg/xen/common/domain.c
@@ -28,6 +28,7 @@
 #include <asm/debugger.h>
 #include <public/sched.h>
 #include <public/vcpu.h>
+#include <acm/acm_hooks.h>
 
 /* Protect updates/reads (resp.) of domain_list and domain_hash. */
 DEFINE_SPINLOCK(domlist_update_lock);
@@ -178,7 +179,7 @@ struct vcpu *alloc_idle_vcpu(unsigned in
         return v;
 
     d = (vcpu_id == 0) ?
-        domain_create(IDLE_DOMAIN_ID, 0) :
+        domain_create(IDLE_DOMAIN_ID, 0, 0) :
         idle_vcpu[cpu_id - vcpu_id]->domain;
     BUG_ON(d == NULL);
 
@@ -188,7 +189,8 @@ struct vcpu *alloc_idle_vcpu(unsigned in
     return v;
 }
 
-struct domain *domain_create(domid_t domid, unsigned int domcr_flags)
+struct domain *domain_create(domid_t domid, unsigned int domcr_flags,
+                             ssidref_t ssidref)
 {
     struct domain *d, **pd;
 
@@ -210,18 +212,21 @@ struct domain *domain_create(domid_t dom
 
         if ( grant_table_create(d) != 0 )
             goto fail2;
+
+        if ( acm_domain_create(d, ssidref) != 0 )
+            goto fail3;
     }
 
     if ( arch_domain_create(d) != 0 )
-        goto fail3;
+        goto fail4;
 
     d->iomem_caps = rangeset_new(d, "I/O Memory", RANGESETF_prettyprint_hex);
     d->irq_caps   = rangeset_new(d, "Interrupts", 0);
     if ( (d->iomem_caps == NULL) || (d->irq_caps == NULL) )
-        goto fail4;
+        goto fail5;
 
     if ( sched_init_domain(d) != 0 )
-        goto fail4;
+        goto fail5;
 
     if ( !is_idle_domain(d) )
     {
@@ -243,8 +248,11 @@ struct domain *domain_create(domid_t dom
 
     return d;
 
- fail4:
+ fail5:
     arch_domain_destroy(d);
+ fail4:
+    if ( !is_idle_domain(d) )
+        acm_domain_destroy(d);
  fail3:
     if ( !is_idle_domain(d) )
         grant_table_destroy(d);
@@ -313,6 +321,7 @@ void domain_kill(struct domain *d)
         return;
     }
 
+    acm_domain_destroy(d);
     gnttab_release_mappings(d);
     domain_relinquish_resources(d);
     put_domain(d);
Index: root/xen-unstable.hg/xen/include/acm/acm_hooks.h
===================================================================
--- root.orig/xen-unstable.hg/xen/include/acm/acm_hooks.h
+++ root/xen-unstable.hg/xen/include/acm/acm_hooks.h
@@ -96,10 +96,9 @@ struct acm_operations {
     int  (*dump_statistics)            (u8 *buffer, u16 buf_size);
     int  (*dump_ssid_types)            (ssidref_t ssidref, u8 *buffer, u16 buf_size);
     /* domain management control hooks (can be NULL) */
-    int  (*pre_domain_create)          (void *subject_ssid, ssidref_t ssidref);
-    void (*post_domain_create)         (domid_t domid, ssidref_t ssidref);
-    void (*fail_domain_create)         (void *subject_ssid, ssidref_t ssidref);
-    void (*post_domain_destroy)        (void *object_ssid, domid_t id);
+    int  (*domain_create)              (void *subject_ssid, ssidref_t ssidref,
+                                        domid_t domid);
+    void (*domain_destroy)             (void *object_ssid, struct domain *d);
     /* event channel control hooks  (can be NULL) */
     int  (*pre_eventchannel_unbound)      (domid_t id1, domid_t id2);
     void (*fail_eventchannel_unbound)     (domid_t id1, domid_t id2);
@@ -128,14 +127,9 @@ extern struct acm_operations *acm_second
 # define traceprintk(fmt, args...)
 #endif
 
+
 #ifndef ACM_SECURITY
 
-static inline int acm_pre_domctl(struct xen_domctl *op, void **ssid) 
-{ return 0; }
-static inline void acm_post_domctl(struct xen_domctl *op, void *ssid) 
-{ return; }
-static inline void acm_fail_domctl(struct xen_domctl *op, void *ssid) 
-{ return; }
 static inline int acm_pre_eventchannel_unbound(domid_t id1, domid_t id2)
 { return 0; }
 static inline int acm_pre_eventchannel_interdomain(domid_t id)
@@ -148,53 +142,17 @@ static inline int acm_init(char *policy_
 { return 0; }
 static inline int acm_is_policy(char *buf, unsigned long len)
 { return 0; }
-static inline void acm_post_domain0_create(domid_t domid) 
-{ return; }
 static inline int acm_sharing(ssidref_t ssidref1, ssidref_t ssidref2)
 { return 0; }
+static inline int acm_domain_create(struct domain *d, ssidref_t ssidref)
+{ return 0; }
+static inline void acm_domain_destroy(struct domain *d)
+{ return; }
 
-#else
-
-static inline int acm_pre_domain_create(void *subject_ssid, ssidref_t ssidref)
-{
-    if ((acm_primary_ops->pre_domain_create != NULL) && 
-        acm_primary_ops->pre_domain_create(subject_ssid, ssidref))
-        return ACM_ACCESS_DENIED;
-    else if ((acm_secondary_ops->pre_domain_create != NULL) && 
-             acm_secondary_ops->pre_domain_create(subject_ssid, ssidref)) {
-        /* roll-back primary */
-        if (acm_primary_ops->fail_domain_create != NULL)
-            acm_primary_ops->fail_domain_create(subject_ssid, ssidref);
-        return ACM_ACCESS_DENIED;
-    } else
-        return ACM_ACCESS_PERMITTED;
-}
-
-static inline void acm_post_domain_create(domid_t domid, ssidref_t ssidref)
-{
-    if (acm_primary_ops->post_domain_create != NULL)
-        acm_primary_ops->post_domain_create(domid, ssidref);
-    if (acm_secondary_ops->post_domain_create != NULL)
-        acm_secondary_ops->post_domain_create(domid, ssidref);
-}
+#define DOM0_SSIDREF 0x0
 
-static inline void acm_fail_domain_create(
-    void *subject_ssid, ssidref_t ssidref)
-{
-    if (acm_primary_ops->fail_domain_create != NULL)
-        acm_primary_ops->fail_domain_create(subject_ssid, ssidref);
-    if (acm_secondary_ops->fail_domain_create != NULL)
-        acm_secondary_ops->fail_domain_create(subject_ssid, ssidref);
-}
+#else
 
-static inline void acm_post_domain_destroy(void *object_ssid, domid_t id)
-{
-    if (acm_primary_ops->post_domain_destroy != NULL)
-        acm_primary_ops->post_domain_destroy(object_ssid, id);
-    if (acm_secondary_ops->post_domain_destroy != NULL)
-        acm_secondary_ops->post_domain_destroy(object_ssid, id);
-    return;
-}
 
 static inline int acm_pre_eventchannel_unbound(domid_t id1, domid_t id2)
 {
@@ -226,85 +184,6 @@ static inline int acm_pre_eventchannel_i
         return ACM_ACCESS_PERMITTED;
 }
 
-static inline int acm_pre_domctl(struct xen_domctl *op, void **ssid) 
-{
-    int ret = -EACCES;
-    struct domain *d;
-
-    switch(op->cmd) {
-    case XEN_DOMCTL_createdomain:
-        ret = acm_pre_domain_create(
-            current->domain->ssid, op->u.createdomain.ssidref);
-        break;
-    case XEN_DOMCTL_destroydomain:
-        if (*ssid != NULL) {
-            printkd("%s: Warning. Overlapping destruction.\n", 
-                    __func__);
-            return -EACCES;
-        }
-        d = rcu_lock_domain_by_id(op->domain);
-        if (d != NULL) {
-            *ssid = d->ssid; /* save for post destroy when d is gone */
-            if (*ssid == NULL) {
-                printk("%s: Warning. Destroying domain without ssid pointer.\n", 
-                       __func__);
-                rcu_unlock_domain(d);
-                return -EACCES;
-            }
-            d->ssid = NULL; /* make sure it's not used any more */
-             /* no policy-specific hook */
-            rcu_unlock_domain(d);
-            ret = 0;
-        }
-        break;
-    default:
-        ret = 0; /* ok */
-    }
-    return ret;
-}
-
-static inline void acm_post_domctl(struct xen_domctl *op, void **ssid)
-{
-    switch(op->cmd) {
-    case XEN_DOMCTL_createdomain:
-        /* initialialize shared sHype security labels for new domain */
-        acm_init_domain_ssid(
-            op->domain, op->u.createdomain.ssidref);
-        acm_post_domain_create(
-            op->domain, op->u.createdomain.ssidref);
-        break;
-    case XEN_DOMCTL_destroydomain:
-        if (*ssid == NULL) {
-            printkd("%s: ERROR. SSID unset.\n",
-                    __func__);
-            break;
-        }
-        acm_post_domain_destroy(*ssid, op->domain);
-        /* free security ssid for the destroyed domain (also if null policy */
-        acm_free_domain_ssid((struct acm_ssid_domain *)(*ssid));
-        *ssid = NULL;
-        break;
-    }
-}
-
-static inline void acm_fail_domctl(struct xen_domctl *op, void **ssid)
-{
-    switch(op->cmd) {
-    case XEN_DOMCTL_createdomain:
-        acm_fail_domain_create(
-            current->domain->ssid, op->u.createdomain.ssidref);
-        break;
-    case XEN_DOMCTL_destroydomain:
-        /*  we don't handle domain destroy failure but at least free the ssid */
-        if (*ssid == NULL) {
-            printkd("%s: ERROR. SSID unset.\n",
-                    __func__);
-            break;
-        }
-        acm_free_domain_ssid((struct acm_ssid_domain *)(*ssid));
-        *ssid = NULL;
-    }
-}
 
 static inline int acm_pre_grant_map_ref(domid_t id)
 {
@@ -348,15 +227,52 @@ static inline int acm_pre_grant_setup(do
     }
 }
 
-static inline void acm_post_domain0_create(domid_t domid)
+
+static inline int acm_domain_create(struct domain *d, ssidref_t ssidref)
 {
-    /* initialialize shared sHype security labels for new domain */
-    int dom0_ssidref = dom0_ste_ssidref << 16 | dom0_chwall_ssidref;
+    void *subject_ssid = current->domain->ssid;
+    domid_t domid = d->domain_id;
+    int rc;
+
+    /*
+       To be called when a domain is created; returns '0' if the
+       domain is allowed to be created, != '0' if not.
+     */
+    rc = acm_init_domain_ssid_new(d, ssidref);
+    if (rc != ACM_OK)
+        return rc;
+
+    if ((acm_primary_ops->domain_create != NULL) &&
+        acm_primary_ops->domain_create(subject_ssid, ssidref, domid)) {
+        return ACM_ACCESS_DENIED;
+    } else if ((acm_secondary_ops->domain_create != NULL) &&
+                acm_secondary_ops->domain_create(subject_ssid, ssidref,
+                                                 domid)) {
+        /* roll-back primary */
+        if (acm_primary_ops->domain_destroy != NULL)
+            acm_primary_ops->domain_destroy(d->ssid, d);
+        acm_free_domain_ssid(d->ssid);
+        return ACM_ACCESS_DENIED;
+    }
+
+    return 0;
+}
 
-    acm_init_domain_ssid(domid, dom0_ssidref);
-    acm_post_domain_create(domid, dom0_ssidref);
+
+static inline void acm_domain_destroy(struct domain *d)
+{
+    void *ssid = d->ssid;
+    if (ssid != NULL) {
+        if (acm_primary_ops->domain_destroy != NULL)
+            acm_primary_ops->domain_destroy(ssid, d);
+        if (acm_secondary_ops->domain_destroy != NULL)
+            acm_secondary_ops->domain_destroy(ssid, d);
+        /* free security ssid for the destroyed domain (also if null policy */
+        acm_free_domain_ssid((struct acm_ssid_domain *)(ssid));
+    }
 }
 
+
 static inline int acm_sharing(ssidref_t ssidref1, ssidref_t ssidref2)
 {
     if ((acm_primary_ops->sharing != NULL) &&
@@ -375,6 +291,8 @@ extern int acm_init(char *policy_start, 
 /* Return true iff buffer has an acm policy magic number.  */
 extern int acm_is_policy(char *buf, unsigned long len);
 
+#define DOM0_SSIDREF (dom0_ste_ssidref << 16 | dom0_chwall_ssidref)
+
 #endif
 
 #endif
Index: root/xen-unstable.hg/xen/include/xen/sched.h
===================================================================
--- root.orig/xen-unstable.hg/xen/include/xen/sched.h
+++ root/xen-unstable.hg/xen/include/xen/sched.h
@@ -10,6 +10,7 @@
 #include <public/xen.h>
 #include <public/domctl.h>
 #include <public/vcpu.h>
+#include <public/acm.h>
 #include <xen/time.h>
 #include <xen/timer.h>
 #include <xen/grant_table.h>
@@ -296,7 +297,8 @@ static inline struct domain *get_current
     return d;
 }
 
-struct domain *domain_create(domid_t domid, unsigned int domcr_flags);
+struct domain *domain_create(domid_t domid, unsigned int domcr_flags,
+                             ssidref_t ssidref);
  /* DOMCRF_hvm: Create an HVM domain, as opposed to a PV domain. */
 #define _DOMCRF_hvm 0
 #define DOMCRF_hvm  (1U<<_DOMCRF_hvm)
Index: root/xen-unstable.hg/xen/arch/ia64/xen/xensetup.c
===================================================================
--- root.orig/xen-unstable.hg/xen/arch/ia64/xen/xensetup.c
+++ root/xen-unstable.hg/xen/arch/ia64/xen/xensetup.c
@@ -421,7 +421,7 @@ void start_kernel(void)
 
     scheduler_init();
     idle_vcpu[0] = (struct vcpu*) ia64_r13;
-    idle_domain = domain_create(IDLE_DOMAIN_ID, 0);
+    idle_domain = domain_create(IDLE_DOMAIN_ID, 0, 0);
     if ( (idle_domain == NULL) || (alloc_vcpu(idle_domain, 0, 0) == NULL) )
         BUG();
 
@@ -508,7 +508,7 @@ printk("num_online_cpus=%d, max_cpus=%d\
     expose_p2m_init();
 
     /* Create initial domain 0. */
-    dom0 = domain_create(0, 0);
+    dom0 = domain_create(0, 0, DOM0_SSIDREF);
     if (dom0 == NULL)
         panic("Error creating domain 0\n");
     dom0_vcpu0 = alloc_vcpu(dom0, 0, 0);
Index: root/xen-unstable.hg/xen/arch/powerpc/setup.c
===================================================================
--- root.orig/xen-unstable.hg/xen/arch/powerpc/setup.c
+++ root/xen-unstable.hg/xen/arch/powerpc/setup.c
@@ -162,7 +162,7 @@ static void __init start_of_day(void)
     scheduler_init();
 
     /* create idle domain */
-    idle_domain = domain_create(IDLE_DOMAIN_ID, 0);
+    idle_domain = domain_create(IDLE_DOMAIN_ID, 0, 0);
     if ((idle_domain == NULL) || (alloc_vcpu(idle_domain, 0, 0) == NULL))
         BUG();
     set_current(idle_domain->vcpu[0]);
@@ -370,7 +370,7 @@ static void __init __start_xen(multiboot
     percpu_free_unused_areas();
 
     /* Create initial domain 0. */
-    dom0 = domain_create(0, 0);
+    dom0 = domain_create(0, 0, DOM0_SSIDREF);
     if (dom0 == NULL)
         panic("Error creating domain 0\n");
 
@@ -380,9 +380,6 @@ static void __init __start_xen(multiboot
 
     dom0->is_privileged = 1;
 
-    /* Post-create hook sets security label. */
-    acm_post_domain0_create(dom0->domain_id);
-
     cmdline = (char *)(mod[0].string ? __va((ulong)mod[0].string) : NULL);
 
     /* scrub_heap_pages() requires IRQs enabled, and we're post IRQ setup... */
Index: root/xen-unstable.hg/xen/acm/acm_null_hooks.c
===================================================================
--- root.orig/xen-unstable.hg/xen/acm/acm_null_hooks.c
+++ root/xen-unstable.hg/xen/acm/acm_null_hooks.c
@@ -62,10 +62,8 @@ struct acm_operations acm_null_ops = {
     .dump_statistics = null_dump_stats,
     .dump_ssid_types = null_dump_ssid_types,
     /* domain management control hooks */
-    .pre_domain_create = NULL,
-    .post_domain_create = NULL,
-    .fail_domain_create = NULL,
-    .post_domain_destroy = NULL,
+    .domain_create = NULL,
+    .domain_destroy = NULL,
     /* event channel control hooks */
     .pre_eventchannel_unbound = NULL,
     .fail_eventchannel_unbound = NULL,
Index: root/xen-unstable.hg/xen/acm/acm_chinesewall_hooks.c
===================================================================
--- root.orig/xen-unstable.hg/xen/acm/acm_chinesewall_hooks.c
+++ root/xen-unstable.hg/xen/acm/acm_chinesewall_hooks.c
@@ -407,26 +407,23 @@ static int chwall_dump_ssid_types(ssidre
 
 /* -------- DOMAIN OPERATION HOOKS -----------*/
 
-static int chwall_pre_domain_create(void *subject_ssid, ssidref_t ssidref)
+static int _chwall_pre_domain_create(void *subject_ssid, ssidref_t ssidref)
 {
     ssidref_t chwall_ssidref;
     int i, j;
     traceprintk("%s.\n", __func__);
 
-    read_lock(&acm_bin_pol_rwlock);
     chwall_ssidref = GET_SSIDREF(ACM_CHINESE_WALL_POLICY, ssidref);
     if (chwall_ssidref == ACM_DEFAULT_LOCAL_SSID)
     {
         printk("%s: ERROR CHWALL SSID is NOT SET but policy enforced.\n",
                __func__);
-        read_unlock(&acm_bin_pol_rwlock);
         return ACM_ACCESS_DENIED;       /* catching and indicating config error */
     }
     if (chwall_ssidref >= chwall_bin_pol.max_ssidrefs)
     {
         printk("%s: ERROR chwall_ssidref > max(%x).\n",
                __func__, chwall_bin_pol.max_ssidrefs - 1);
-        read_unlock(&acm_bin_pol_rwlock);
         return ACM_ACCESS_DENIED;
     }
     /* A: chinese wall check for conflicts */
@@ -436,7 +433,6 @@ static int chwall_pre_domain_create(void
                                    chwall_bin_pol.max_types + i])
         {
             printk("%s: CHINESE WALL CONFLICT in type %02x.\n", __func__, i);
-            read_unlock(&acm_bin_pol_rwlock);
             return ACM_ACCESS_DENIED;
         }
 
@@ -465,17 +461,16 @@ static int chwall_pre_domain_create(void
                                            chwall_bin_pol.max_types + j])
                 chwall_bin_pol.conflict_aggregate_set[j]++;
     }
-    read_unlock(&acm_bin_pol_rwlock);
     return ACM_ACCESS_PERMITTED;
 }
 
-static void chwall_post_domain_create(domid_t domid, ssidref_t ssidref)
+
+static void _chwall_post_domain_create(domid_t domid, ssidref_t ssidref)
 {
     int i, j;
     ssidref_t chwall_ssidref;
     traceprintk("%s.\n", __func__);
 
-    read_lock(&acm_bin_pol_rwlock);
     chwall_ssidref = GET_SSIDREF(ACM_CHINESE_WALL_POLICY, ssidref);
     /* adjust types ref-count for running domains */
     for (i = 0; i < chwall_bin_pol.max_types; i++)
@@ -484,7 +479,6 @@ static void chwall_post_domain_create(do
                                    chwall_bin_pol.max_types + i];
     if (domid)
     {
-        read_unlock(&acm_bin_pol_rwlock);
         return;
     }
     /* Xen does not call pre-create hook for DOM0;
@@ -519,48 +513,34 @@ static void chwall_post_domain_create(do
                                            chwall_bin_pol.max_types + j])
                 chwall_bin_pol.conflict_aggregate_set[j]++;
     }
-    read_unlock(&acm_bin_pol_rwlock);
     return;
 }
 
-static void
-chwall_fail_domain_create(void *subject_ssid, ssidref_t ssidref)
-{
-    int i, j;
-    ssidref_t chwall_ssidref;
-    traceprintk("%s.\n", __func__);
 
+/*
+ * To be called when creating a domain. If this call is unsuccessful,
+ * no state changes have occurred (adjustments of counters etc.). If it
+ * was successful, state was changed and can be undone using
+ * chwall_domain_destroy.
+ */
+static int chwall_domain_create(void *subject_ssid, ssidref_t ssidref,
+                                domid_t domid)
+{
+    int rc;
     read_lock(&acm_bin_pol_rwlock);
-    chwall_ssidref = GET_SSIDREF(ACM_CHINESE_WALL_POLICY, ssidref);
-    /* roll-back: re-adjust conflicting types aggregate */
-    for (i = 0; i < chwall_bin_pol.max_conflictsets; i++)
-    {
-        int common = 0;
-        /* check if conflict_set_i and ssidref have common types */
-        for (j = 0; j < chwall_bin_pol.max_types; j++)
-            if (chwall_bin_pol.
-                conflict_sets[i * chwall_bin_pol.max_types + j]
-                && chwall_bin_pol.ssidrefs[chwall_ssidref *
-                                          chwall_bin_pol.max_types + j])
-            {
-                common = 1;
-                break;
-            }
-        if (common == 0)
-            continue;           /* try next conflict set, this one does not include any type of chwall_ssidref */
-        /* now add types of the conflict set to conflict_aggregate_set (except types in chwall_ssidref) */
-        for (j = 0; j < chwall_bin_pol.max_types; j++)
-            if (chwall_bin_pol.
-                conflict_sets[i * chwall_bin_pol.max_types + j]
-                && !chwall_bin_pol.ssidrefs[chwall_ssidref *
-                                           chwall_bin_pol.max_types + j])
-                chwall_bin_pol.conflict_aggregate_set[j]--;
+    rc = _chwall_pre_domain_create(subject_ssid, ssidref);
+    if (rc == ACM_ACCESS_PERMITTED) {
+        _chwall_post_domain_create(domid, ssidref);
     }
     read_unlock(&acm_bin_pol_rwlock);
+    return rc;
 }
 
-
-static void chwall_post_domain_destroy(void *object_ssid, domid_t id)
+/*
+ * This function undoes everything a successful call to
+ * chwall_domain_create has done.
+ */
+static void chwall_domain_destroy(void *object_ssid, struct domain *d)
 {
     int i, j;
     struct chwall_ssid *chwall_ssidp = GET_SSIDP(ACM_CHINESE_WALL_POLICY,
@@ -614,10 +594,8 @@ struct acm_operations acm_chinesewall_op
     .dump_statistics = chwall_dump_stats,
     .dump_ssid_types = chwall_dump_ssid_types,
     /* domain management control hooks */
-    .pre_domain_create = chwall_pre_domain_create,
-    .post_domain_create = chwall_post_domain_create,
-    .fail_domain_create = chwall_fail_domain_create,
-    .post_domain_destroy = chwall_post_domain_destroy,
+    .domain_create = chwall_domain_create,
+    .domain_destroy = chwall_domain_destroy,
     /* event channel control hooks */
     .pre_eventchannel_unbound = NULL,
     .fail_eventchannel_unbound = NULL,
Index: root/xen-unstable.hg/xen/acm/acm_simple_type_enforcement_hooks.c
===================================================================
--- root.orig/xen-unstable.hg/xen/acm/acm_simple_type_enforcement_hooks.c
+++ root/xen-unstable.hg/xen/acm/acm_simple_type_enforcement_hooks.c
@@ -500,11 +500,18 @@ ste_pre_domain_create(void *subject_ssid
     return ACM_ACCESS_PERMITTED;
 }
 
+static int
+ste_domain_create(void *subject_ssid, ssidref_t ssidref, domid_t  domid)
+{
+    return ste_pre_domain_create(subject_ssid, ssidref);
+}
+
+
 static void 
-ste_post_domain_destroy(void *subject_ssid, domid_t id)
+ste_domain_destroy(void *subject_ssid, struct domain *d)
 {
     /* clean all cache entries for destroyed domain (might be re-used) */
-    clean_id_from_cache(id);
+    clean_id_from_cache(d->domain_id);
 }
 
 /* -------- EVENTCHANNEL OPERATIONS -----------*/
@@ -685,10 +692,8 @@ struct acm_operations acm_simple_type_en
     .dump_ssid_types        = ste_dump_ssid_types,
 
     /* domain management control hooks */
-    .pre_domain_create       = ste_pre_domain_create,
-    .post_domain_create     = NULL,
-    .fail_domain_create     = NULL,
-    .post_domain_destroy    = ste_post_domain_destroy,
+    .domain_create = ste_domain_create,
+    .domain_destroy    = ste_domain_destroy,
 
     /* event channel control hooks */
     .pre_eventchannel_unbound   = ste_pre_eventchannel_unbound,

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] [Xen] [ACM] Code restructuring

[Keir Fraser]

[-- Attachment #1.1: Type: text/plain, Size: 749 bytes --]

On 24/4/07 17:15, "Stefan Berger" <stefanb@us.ibm.com> wrote:

>> > Much nicer, thank you. I've applied it for 3.0.5. However, given the effort
>> > put in to labelling a domain at the very start of its lifetime, wouldn't it
>> > be a good idea to leave that label attached until the domain is actually
>> > destroyed? This would correspond to putting the destructor call in
>> > domain_destroy() or, even better, complete_domain_destroy(). Bear in mind
>> > that a domain may still be accessible and have resources allocated to it
>> > even after domain_kill() finishes executing. So stripping its resource
>> label
>> > at that point in time is a bit worrying.
> 
> Right. Can you move this one call into complete_domain_destroy()?

Will do.
 K.


[-- Attachment #1.2: Type: text/html, Size: 1368 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel