From: Reiner Sailer <sailer@us.ibm.com>
To: Keir Fraser <Keir.Fraser@cl.cam.ac.uk>,
Xen Mailing List <xen-devel@lists.xensource.com>
Cc: Rusty Russell <rusty@rustcorp.com.au>,
Tony Breeds <tony@bakeyournoodle.com>
Subject: [PATCH] acm_ops.c cleanup
Date: Fri, 25 Nov 2005 00:32:04 -0500 [thread overview]
Message-ID: <4386A1D4.5090308@us.ibm.com> (raw)
[-- Attachment #1: Type: text/plain, Size: 907 bytes --]
Keir Fraser <Keir.Fraser@cl.cam.ac.uk> wrote on 11/24/2005 05:18:28 AM:
>
> On 24 Nov 2005, at 04:48, Tony Breeds wrote:
>
> > Hi All,
> > xen/common/acm_ops.c, check for a NULL pointer and then
> > cheerfully dereferences it.
> >
> > Signed-off-by: Tony Breeds <tony@bakeyournoodle.com>
>
> 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 <sailer@us.ibm.com>
[-- Attachment #2: acm_ops.diff --]
[-- Type: text/plain, Size: 8843 bytes --]
xen/common/acm_ops.c | 179 +++++++++++++++++++++++++--------------------------
1 files changed, 90 insertions(+), 89 deletions(-)
Index: xen-unstable.hg-shype/xen/common/acm_ops.c
===================================================================
--- xen-unstable.hg-shype.orig/xen/common/acm_ops.c
+++ xen-unstable.hg-shype/xen/common/acm_ops.c
@@ -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;
}
[-- Attachment #3: Type: text/plain, Size: 138 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
reply other threads:[~2005-11-25 5:32 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4386A1D4.5090308@us.ibm.com \
--to=sailer@us.ibm.com \
--cc=Keir.Fraser@cl.cam.ac.uk \
--cc=rusty@rustcorp.com.au \
--cc=tony@bakeyournoodle.com \
--cc=xen-devel@lists.xensource.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.