acm_ops.c cleanup
PATCH 1/1 acm_ops.c cleanup
Keir Fraser <[email protected]> wrote on 11/24/2005 05:18:28 AM:
Quoted 7 lines · Tony Breeds
Quoted 5 lines
Hi All,
xen/common/acm_ops.c, check for a NULL pointer and then
cheerfully dereferences it.
Quoted 8 lines
Quoted 1 line
Signed-off-by Tony Breeds <[email protected]>Applied, but nearby code goto's out so I changed the patch to do that.
In fact there seems to be no convention in that function as to whether returning immediately or goto'ing out (and doing dome return-code cooking) is the right thing to do. Should all the returns be goto out's?
I cleaned this file up and elimnated 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.
Thanks
Reiner
Signed-off: Reiner Sailer <[email protected]>
xen/common/acm_ops.c | 179 +++++++++++++++++++++++++-------------------------- 1 files changed, 90 insertions(+), 89 deletions(-)
xen/common/acm_ops.c +90 -89
@@ -49,15 +49,11 @@ enum acm_operation {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 do_acm_op(struct acm_op * u_acm_op)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 @@ long do_acm_op(struct acm_op * u_acm_op) {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 @@ long do_acm_op(struct acm_op * u_acm_op) {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 @@ long do_acm_op(struct acm_op * u_acm_op) {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; +break ; } -if (subj->ssid ==NULL ) { +if (subj->ssid ==NULL ) + {put_domain (subj); ret = -ESRCH ; - goto out; +break ; } ssidref1 = ((struct acm_ssid_domain *)(subj->ssid))->ssidref;put_domain (subj); - }else { + } +else + { ret = -ESRCH ; - goto out; +break ; } -if (op->u.getdecision.get_decision_by2 ==SSIDREF ) { +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; +break ;; } -if (subj->ssid ==NULL ) { +if (subj->ssid ==NULL ) + {put_domain (subj); -return -ESRCH ; + ret = -ESRCH ; +break ; } ssidref2 = ((struct acm_ssid_domain *)(subj->ssid))->ssidref;put_domain (subj); - }else { + } +else + { ret = -ESRCH ; - goto out; +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 @@ long do_acm_op(struct acm_op * u_acm_op) 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-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel