Stefan Berger
2007-Apr-21 23:02 UTC
[Xen-devel] [PATCH] [XEN] [ACM] [2/2] Restructuring ACM-related code in do_domctl
This patch restructures the ACM-related code in the do_domctl() function so that the lock to the policy can be fetched in the individual operations, i.e., XEN_DOMCTL_createdomain, and the pair of locking functions acm_rlock_policy()/acm_runlock_policy() don''t take the function parameter anymore. Sign-off-by: Stefan Berger <stefanb@us.ibm.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2007-Apr-22 10:07 UTC
Re: [Xen-devel] [PATCH] [XEN] [ACM] [2/2] Restructuring ACM-related code in do_domctl
I think that relying on domctl wrapper functions to do necessary label bookkeeping on domain create/destroy is making things more complicated than they need to be. Wrapper functions are obviously the right answer for most ACM functions (since they are allow/deny checks, external to the core logic of the operation being checked), but in the case of domain create/destroy ACM is more part of the process. So, how about having functions: domain_acm_create() and domain_acm_destroy()? The former would be called from domain_create(), the latter from the error path of domain_create() and from domain_destroy(). You can then have appropriate critical regions *within* those functions (not across them) that you synchronise against when relabelling the world. And you shouldn''t then need to modify do_domctl() at all? -- Keir On 22/4/07 00:02, "Stefan Berger" <stefanb@us.ibm.com> wrote:> This patch restructures the ACM-related code in the do_domctl() function > so that the lock to the policy can be fetched in the individual > operations, i.e., XEN_DOMCTL_createdomain, and the pair of locking > functions acm_rlock_policy()/acm_runlock_policy() don''t take the > function parameter anymore. > > Sign-off-by: Stefan Berger <stefanb@us.ibm.com> > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefan Berger
2007-Apr-22 17:35 UTC
Re: [Xen-devel] [PATCH] [XEN] [ACM] [2/2] Restructuring ACM-related code in do_domctl
xen-devel-bounces@lists.xensource.com wrote on 04/22/2007 06:07:05 AM:> I think that relying on domctl wrapper functions to do necessary label > bookkeeping on domain create/destroy is making things more complicatedthan> they need to be. Wrapper functions are obviously the right answer formost> ACM functions (since they are allow/deny checks, external to the corelogic> of the operation being checked), but in the case of domaincreate/destroy> ACM is more part of the process. > > So, how about having functions: domain_acm_create() and > domain_acm_destroy()? The former would be called from domain_create(),the> latter from the error path of domain_create() and from domain_destroy(). > > You can then have appropriate critical regions *within* those functions(not> across them) that you synchronise against when relabelling the world.And> you shouldn''t then need to modify do_domctl() at all?At the beginning of do_domctl() there''s the call to acm_pre_domctl, which ends up in its callpath in chwall_pre_domain_create to check whether under the current policy the domain is allowed to be created and it grabs the lock to the policy before doing that. At the end of the do_domctl() there''s the call to acm_post_domctl, in case everything went fine with creating the domain for example. Here it ends up in its callpath in chwall_post_domain_create where it again grabs the lock to the policy and under the assumption that the policy hasn''t changed modifies a counter array (running_types). If the policy is changed in between those calls, i.e. the chinese wall part is changed such that the domain would not be allowed to exist under the new policy, the post-domain-create call would still go through. That''s what I want to prevent with a continously-held lock that spans the evaluation at the beginning and the modification of that counter array at the end. Stefan> > -- Keir > > On 22/4/07 00:02, "Stefan Berger" <stefanb@us.ibm.com> wrote: > > > This patch restructures the ACM-related code in the do_domctl()function> > so that the lock to the policy can be fetched in the individual > > operations, i.e., XEN_DOMCTL_createdomain, and the pair of locking > > functions acm_rlock_policy()/acm_runlock_policy() don''t take the > > function parameter anymore. > > > > Sign-off-by: Stefan Berger <stefanb@us.ibm.com> > > > > _______________________________________________ > > Xen-devel mailing list > > Xen-devel@lists.xensource.com > > http://lists.xensource.com/xen-devel > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2007-Apr-22 18:34 UTC
Re: [Xen-devel] [PATCH] [XEN] [ACM] [2/2] Restructuring ACM-related code in do_domctl
On 22/4/07 18:35, "Stefan Berger" <stefanb@us.ibm.com> wrote:> At the beginning of do_domctl() there''s the call to acm_pre_domctl, which ends > up in its callpath in chwall_pre_domain_create to check whether under the > current policy the domain is allowed to be created and it grabs the lock to > the policy before doing that. > At the end of the do_domctl() there''s the call to acm_post_domctl, in case > everything went fine with creating the domain for example. Here it ends up in > its callpath in chwall_post_domain_create where it again grabs the lock to the > policy and under the assumption that the policy hasn''t changed modifies a > counter array (running_types). > If the policy is changed in between those calls, i.e. the chinese wall part is > changed such that the domain would not be allowed to exist under the new > policy, the post-domain-create call would still go through. That''s what I want > to prevent with a continously-held lock that spans the evaluation at the > beginning and the modification of that counter array at the end.If you did this with a straightforward domain_create() hook, you could update state at the same time as doing the policy check. Your domain_destroy() hook would be called if the creation subsequently failed to commit. And if the policy changes at any time after the call to your domain_create() hook, you¹ve already updated your ACM state so you can see the new domain via some internal structure you presumably maintains, and hence can re-evaluate the decision under the new policy. In general, keeping checks and state updates together is nice compared with pushing them to pre/post hooks with locks implicitly held across the two. That¹s just plain gross imo. That is, an architecture where you have a pre-doing-stuff¹ hook and a pre-destroying-stuff¹ hook, where the latter is also called when the doing-stuff action turns out to fail, is nicer than pre/post hooks. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefan Berger
2007-Apr-23 03:37 UTC
Re: [Xen-devel] [PATCH] [XEN] [ACM] [2/2] Restructuring ACM-related code in do_domctl
Keir Fraser <Keir.Fraser@cl.cam.ac.uk> wrote on 04/22/2007 02:34:00 PM:> On 22/4/07 18:35, "Stefan Berger" <stefanb@us.ibm.com> wrote:> At the beginning of do_domctl() there''s the call to acm_pre_domctl, > which ends up in its callpath in chwall_pre_domain_create to check > whether under the current policy the domain is allowed to be created > and it grabs the lock to the policy before doing that. > At the end of the do_domctl() there''s the call to acm_post_domctl, > in case everything went fine with creating the domain for example. > Here it ends up in its callpath in chwall_post_domain_create where > it again grabs the lock to the policy and under the assumption that > the policy hasn''t changed modifies a counter array (running_types). > If the policy is changed in between those calls, i.e. the chinese > wall part is changed such that the domain would not be allowed to > exist under the new policy, the post-domain-create call would still > go through. That''s what I want to prevent with a continously-held > lock that spans the evaluation at the beginning and the modification > of that counter array at the end. > > If you did this with a straightforward domain_create() hook, you > could update state at the same time as doing the policy check. Your > domain_destroy() hook would be called if the creation subsequently > failed to commit. And if the policy changes at any time after the > call to your domain_create() hook, you?ve already updated your ACM > state so you can see the new domain via some internal structure you > presumably maintains, and hence can re-evaluate the decision under > the new policy.The ''internal structure'' that represents the label of the domain is maintained through a pointer connected to the domain structure. For that reason I would intend to hold the lock on the policy until the domain has been added to the list of domains.> > In general, keeping checks and state updates together is nice > compared with pushing them to pre/post hooks with locks implicitly > held across the two. That?s just plain gross imo. > > That is, an architecture where you have a ?pre-doing-stuff? hook and > a ?pre-destroying-stuff? hook, where the latter is also called when > the doing-stuff action turns out to fail, is nicer than pre/post hooks.I''ll change that in do_domctl(). Stefan> > -- Keir_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2007-Apr-23 07:59 UTC
Re: [Xen-devel] [PATCH] [XEN] [ACM] [2/2] Restructuring ACM-related code in do_domctl
On 23/4/07 04:37, "Stefan Berger" <stefanb@us.ibm.com> wrote:> The ''internal structure'' that represents the label of the domain is maintained > through a pointer connected to the domain structure. For that reason I would > intend to hold the lock on the policy until the domain has been added to the > list of domains. >In this case it probably makes sense to maintain your own domain list within your ACM subsystem, so that you can always find any domain that has been registered with you via your domain_create hook. I suggest a policy-specific structure pointed at by struct domain (void *security ?). You can dynamically allocate this struct on domain_create() and it can contain a list_head to chain domains through. You can also hide SSID in there (it¹s going to need to move out of generic struct domain when XSM comes along, as sHype won¹t be the only game in town any longer).> >> > That is, an architecture where you have a pre-doing-stuff¹ hook and >> > a pre-destroying-stuff¹ hook, where the latter is also called when >> > the doing-stuff action turns out to fail, is nicer than pre/post hooks. > > I''ll change that in do_domctl().If you do it in domain_create() itself you¹ll also get your hook called for creation of dom0. That will probably avoid some nasty hacking around (like you have right now in chwall_post_domain_create, because the pre-hook is missed out for dom0). -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
George S. Coker, II
2007-Apr-23 16:33 UTC
Re: [Xen-devel] [PATCH] [XEN] [ACM] [2/2] Restructuring ACM-related code in do_domctl
On Sun, 2007-04-22 at 11:07 +0100, Keir Fraser wrote:> I think that relying on domctl wrapper functions to do necessary label > bookkeeping on domain create/destroy is making things more complicated than > they need to be. Wrapper functions are obviously the right answer for most > ACM functions (since they are allow/deny checks, external to the core logic > of the operation being checked), but in the case of domain create/destroy > ACM is more part of the process. > > So, how about having functions: domain_acm_create() and > domain_acm_destroy()? The former would be called from domain_create(), the > latter from the error path of domain_create() and from domain_destroy(). > > You can then have appropriate critical regions *within* those functions (not > across them) that you synchronise against when relabelling the world. And > you shouldn''t then need to modify do_domctl() at all? >I''ll chime in here, although my response applies to later parts of the thread. XSM provides hooks in domain_create and domain_destroy so that modules can do these kinds of operations. There are some side issues that need to be considered in the creation of these hooks, such as when domain_create is first called and when any security module is initialized. In my experience it has been straightforward to factor the existing sHype(ACM) module to take advantage of these additional hooks. A nice side effect (for sHype(ACM)) of the domain_destroy hook is we can clean up the destroy domain operation in do_domctl by removing the local ssid variable as well ensure that the security object is destroyed/freed last. I''ll be posting some updated and cleaned up XSM patches later today/tomorrow. I realize many may be distracted with the 3.0.5 release candidate, but this should help me get XSM into good shape for when 3.0.6 opens up for patches. George _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel