[Xen-changelog] I cleaned up acm_ops.c and eliminated returns inside the switch

[Xen patchbot -unstable <patchbot-unstable@xxxxxxxxxxxxxxxxxxx>]

# HG changeset patch
# User kaf24@xxxxxxxxxxxxxxxxxxxx
# Node ID a8ac8be1a889d633e5d65e3f27a93b4726bfc9a4
# Parent  cb215a84d1af0aa0143e52a459607f54503a51a7
I cleaned up acm_ops.c and eliminated returns inside the switch 
statement. When we need locks, we can place them now around the switch 
statement.

I also included the comments from Rusty and now return -EPERM for denied 
permission errors.

Signed-off: Reiner Sailer <sailer@xxxxxxxxxx>

diff -r cb215a84d1af -r a8ac8be1a889 xen/common/acm_ops.c
--- a/xen/common/acm_ops.c      Fri Nov 25 08:14:01 2005
+++ b/xen/common/acm_ops.c      Fri Nov 25 08:15:08 2005
@@ -49,15 +49,11 @@
 
 int acm_authorize_acm_ops(struct domain *d, enum acm_operation pops)
 {
-    /* all policy management functions are restricted to privileged domains,
-     * soon we will introduce finer-grained privileges for policy operations
-     */
+    /* currently, policy management functions are restricted to privileged 
domains */
     if (!IS_PRIV(d))
-    {
-        printk("%s: ACM management authorization denied ERROR!\n", __func__);
-        return ACM_ACCESS_DENIED;
-    }
-    return ACM_ACCESS_PERMITTED;
+        return -EPERM;
+
+    return 0;
 }
 
 long do_acm_op(struct acm_op * u_acm_op)
@@ -65,10 +61,8 @@
     long ret = 0;
     struct acm_op curop, *op = &curop;
 
-    /* check here policy decision for policy commands */
-    /* for now allow DOM0 only, later indepedently    */
     if (acm_authorize_acm_ops(current->domain, POLICY))
-        return -EACCES;
+        return -EPERM;
 
     if (copy_from_user(op, u_acm_op, sizeof(*op)))
         return -EFAULT;
@@ -80,43 +74,32 @@
     {
     case ACM_SETPOLICY:
     {
-        if (acm_authorize_acm_ops(current->domain, SETPOLICY))
-            return -EACCES;
-        printkd("%s: setting policy.\n", __func__);
-        ret = acm_set_policy(op->u.setpolicy.pushcache,
-                             op->u.setpolicy.pushcache_size, 1);
-        if (ret == ACM_OK)
-            ret = 0;
-        else
-            ret = -ESRCH;
+        ret = acm_authorize_acm_ops(current->domain, SETPOLICY);
+        if (!ret)
+            ret = acm_set_policy(op->u.setpolicy.pushcache,
+                                 op->u.setpolicy.pushcache_size, 1);
     }
     break;
 
     case ACM_GETPOLICY:
     {
-        if (acm_authorize_acm_ops(current->domain, GETPOLICY))
-            return -EACCES;
-        printkd("%s: getting policy.\n", __func__);
-        ret = acm_get_policy(op->u.getpolicy.pullcache,
-                             op->u.getpolicy.pullcache_size);
-        if (ret == ACM_OK)
-            ret = 0;
-        else
-            ret = -ESRCH;
+        ret = acm_authorize_acm_ops(current->domain, GETPOLICY);
+        if (!ret)
+            ret = acm_get_policy(op->u.getpolicy.pullcache,
+                                 op->u.getpolicy.pullcache_size);
+        if (!ret)
+            copy_to_user(u_acm_op, op, sizeof(*op));
     }
     break;
 
     case ACM_DUMPSTATS:
     {
-        if (acm_authorize_acm_ops(current->domain, DUMPSTATS))
-            return -EACCES;
-        printkd("%s: dumping statistics.\n", __func__);
-        ret = acm_dump_statistics(op->u.dumpstats.pullcache,
-                                  op->u.dumpstats.pullcache_size);
-        if (ret == ACM_OK)
-            ret = 0;
-        else
-            ret = -ESRCH;
+        ret = acm_authorize_acm_ops(current->domain, DUMPSTATS);
+        if (!ret)
+            ret = acm_dump_statistics(op->u.dumpstats.pullcache,
+                                      op->u.dumpstats.pullcache_size);
+        if (!ret)
+            copy_to_user(u_acm_op, op, sizeof(*op));
     }
     break;
 
@@ -124,31 +107,39 @@
     {
         ssidref_t ssidref;
 
-        if (acm_authorize_acm_ops(current->domain, GETSSID))
-            return -EACCES;
-        printkd("%s: getting SSID.\n", __func__);
+        ret = acm_authorize_acm_ops(current->domain, GETSSID);
+        if (ret)
+            break;
+
         if (op->u.getssid.get_ssid_by == SSIDREF)
             ssidref = op->u.getssid.id.ssidref;
-        else if (op->u.getssid.get_ssid_by == DOMAINID) {
+        else if (op->u.getssid.get_ssid_by == DOMAINID)
+        {
             struct domain *subj = find_domain_by_id(op->u.getssid.id.domainid);
             if (!subj)
-                return -ESRCH; /* domain not found */
-            if (subj->ssid == NULL) {
+            {
+                ret = -ESRCH; /* domain not found */
+                break;
+            }
+            if (subj->ssid == NULL)
+            {
                 put_domain(subj);
-                return -ESRCH;
+                ret = -ESRCH;
+                break;
             }
             ssidref = ((struct acm_ssid_domain *)(subj->ssid))->ssidref;
             put_domain(subj);
-        } else
-            return -ESRCH;
-
+        }
+        else
+        {
+            ret = -ESRCH;
+            break;
+        }
         ret = acm_get_ssid(ssidref,
                            op->u.getssid.ssidbuf,
                            op->u.getssid.ssidbuf_size);
-        if (ret == ACM_OK)
-            ret = 0;
-        else
-            ret = -ESRCH;
+        if (!ret)
+            copy_to_user(u_acm_op, op, sizeof(*op));
     }
     break;
 
@@ -156,51 +147,75 @@
     {
         ssidref_t ssidref1, ssidref2;
 
-        if (acm_authorize_acm_ops(current->domain, GETDECISION)) {
-            ret = -EACCES;
-            goto out;
-        }
-        printkd("%s: getting access control decision.\n", __func__);
-        if (op->u.getdecision.get_decision_by1 == SSIDREF) {
+        ret = acm_authorize_acm_ops(current->domain, GETDECISION);
+        if (ret)
+            break;
+
+        if (op->u.getdecision.get_decision_by1 == SSIDREF)
             ssidref1 = op->u.getdecision.id1.ssidref;
-        }
-        else if (op->u.getdecision.get_decision_by1 == DOMAINID) {
+        else if (op->u.getdecision.get_decision_by1 == DOMAINID)
+        {
             struct domain *subj = 
find_domain_by_id(op->u.getdecision.id1.domainid);
-            if (!subj) {
+            if (!subj)
+            {
                 ret = -ESRCH; /* domain not found */
-                goto out;
-            }
-            if (subj->ssid == NULL) {
+                break;
+            }
+            if (subj->ssid == NULL)
+            {
                 put_domain(subj);
                 ret = -ESRCH;
-                goto out;
+                break;
             }
             ssidref1 = ((struct acm_ssid_domain *)(subj->ssid))->ssidref;
             put_domain(subj);
-        } else {
-            ret = -ESRCH;
-            goto out;
-        }
-        if (op->u.getdecision.get_decision_by2 == SSIDREF) {
+        }
+        else
+        {
+            ret = -ESRCH;
+            break;
+        }
+        if (op->u.getdecision.get_decision_by2 == SSIDREF)
             ssidref2 = op->u.getdecision.id2.ssidref;
-        }
-        else if (op->u.getdecision.get_decision_by2 == DOMAINID) {
+        else if (op->u.getdecision.get_decision_by2 == DOMAINID)
+        {
             struct domain *subj = 
find_domain_by_id(op->u.getdecision.id2.domainid);
-            if (!subj) {
+            if (!subj)
+            {
                 ret = -ESRCH; /* domain not found */
-                goto out;
-            }
-            if (subj->ssid == NULL) {
+                break;;
+            }
+            if (subj->ssid == NULL)
+            {
                 put_domain(subj);
-                return -ESRCH;
+                ret = -ESRCH;
+                break;
             }
             ssidref2 = ((struct acm_ssid_domain *)(subj->ssid))->ssidref;
             put_domain(subj);
-        } else {
-            ret = -ESRCH;
-            goto out;
+        }
+        else
+        {
+            ret = -ESRCH;
+            break;
         }
         ret = acm_get_decision(ssidref1, ssidref2, op->u.getdecision.hook);
+
+        if (ret == ACM_ACCESS_PERMITTED)
+        {
+            op->u.getdecision.acm_decision = ACM_ACCESS_PERMITTED;
+            ret = 0;
+        }
+        else if  (ret == ACM_ACCESS_DENIED)
+        {
+            op->u.getdecision.acm_decision = ACM_ACCESS_DENIED;
+            ret = 0;
+        }
+        else
+            ret = -ESRCH;
+
+        if (!ret)
+            copy_to_user(u_acm_op, op, sizeof(*op));
     }
     break;
 
@@ -208,20 +223,6 @@
         ret = -ESRCH;
     }
 
- out:
-    if (ret == ACM_ACCESS_PERMITTED) {
-        op->u.getdecision.acm_decision = ACM_ACCESS_PERMITTED;
-        ret = 0;
-    } else if  (ret == ACM_ACCESS_DENIED) {
-        op->u.getdecision.acm_decision = ACM_ACCESS_DENIED;
-        ret = 0;
-    } else {
-        op->u.getdecision.acm_decision = ACM_ACCESS_DENIED;
-        if (ret > 0)
-            ret = -ret;
-    }
-    /* copy decision back to user space */
-    copy_to_user(u_acm_op, op, sizeof(*op));
     return ret;
 }
 

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