[XSM:ACM][PATCH] null dereference bug fix

[XSM:ACM][PATCH] null dereference bug fix

[George S. Coker, II]

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

The attached patch fixes a null dereference bug in XSM:ACM.

Signed-off-by: George Coker <gscoker@alpha.ncsc.mil>

[-- Attachment #2: acm-xsm-null_bug-092707-xen-unstable-15880.diff --]
[-- Type: text/x-patch, Size: 804 bytes --]

diff -r db50ed637dae -r 5cf21d968c59 xen/include/xsm/acm/acm_hooks.h
--- a/xen/include/xsm/acm/acm_hooks.h	Sat Feb 14 15:54:02 2015 -0500
+++ b/xen/include/xsm/acm/acm_hooks.h	Sat Feb 14 15:16:27 2015 -0500
@@ -284,17 +284,12 @@ static inline int acm_domain_create(stru
     } 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);
         rc = ACM_ACCESS_DENIED;
     }
 
     if ( rc == ACM_OK )
     {
         acm_domain_ssid_onto_list(d->ssid);
-    } else {
-        acm_free_domain_ssid(d->ssid);
     }
 
 error_out:

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

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

Re: [Xense-devel] [XSM:ACM][PATCH] null dereference bug fix

[Stefan Berger]

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

xense-devel-bounces@lists.xensource.com wrote on 09/27/2007 12:43:35 PM:

> The attached patch fixes a null dereference bug in XSM:ACM.

As I read this in response to recent error reports - I wonder why CS 15661 
does not expose this error whereas afterwards this error occurs. Are you 
sure this is the right solution? Was something changed in this area of the 
code between 'before XSM' and afterwards?

   Stefan



> 
> Signed-off-by: George Coker <gscoker@alpha.ncsc.mil>
> [attachment "acm-xsm-null_bug-092707-xen-unstable-15880.diff" 
> deleted by Stefan Berger/Watson/IBM] 
> _______________________________________________
> Xense-devel mailing list
> Xense-devel@lists.xensource.com
> http://lists.xensource.com/xense-devel

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

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

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

Re: Re: [Xense-devel] [XSM:ACM][PATCH] nulldereference bug fix

[Coker, George]

This patch is correct for XSM.  The patch creates clean
acm_domain_create and acm_domain_destroy operations.

In 15661 the logic under which acm_domain_destroy is called is slightly
different than under XSM.  In 15661, acm_domain_destroy is called only
if the mask INIT_acm is set.  INIT_acm is set only on successful return
from acm_domain_create.  When acm_domain_create fails, the mask is not
set and acm_domain_destroy is not called.  I do not know if this
resulted in a leak in 15661 due to incomplete cleanup.

George


On Thu, 2007-09-27 at 14:35 -0400, Stefan Berger wrote:
> 
> xense-devel-bounces@lists.xensource.com wrote on 09/27/2007 12:43:35
> PM:
> 
> > The attached patch fixes a null dereference bug in XSM:ACM. 
> 
> As I read this in response to recent error reports - I wonder why CS
> 15661 does not expose this error whereas afterwards this error occurs.
> Are you sure this is the right solution? Was something changed in this
> area of the code between 'before XSM' and afterwards? 
> 
>    Stefan 
> 
> 
> 
> > 
> > Signed-off-by: George Coker <gscoker@alpha.ncsc.mil>
> > [attachment "acm-xsm-null_bug-092707-xen-unstable-15880.diff" 
> > deleted by Stefan Berger/Watson/IBM] 
> > _______________________________________________
> > Xense-devel mailing list
> > Xense-devel@lists.xensource.com
> > http://lists.xensource.com/xense-devel
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
-- 
George S. Coker, II <gscoker@alpha.ncsc.mil> 443-479-6944

Re: Re: [Xense-devel] [XSM:ACM][PATCH] nulldereference bug fix

[Stefan Berger]

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

"Coker, George" <gscoker@tycho.ncsc.mil> wrote on 09/27/2007 03:35:14 PM:

> This patch is correct for XSM.  The patch creates clean
> acm_domain_create and acm_domain_destroy operations.
> 
> In 15661 the logic under which acm_domain_destroy is called is slightly
> different than under XSM.  In 15661, acm_domain_destroy is called only
> if the mask INIT_acm is set.  INIT_acm is set only on successful return
> from acm_domain_create.  When acm_domain_create fails, the mask is not
> set and acm_domain_destroy is not called.  I do not know if this
> resulted in a leak in 15661 due to incomplete cleanup.

So the roll-back call that was necessary before is not necessary anymore?

static inline int acm_domain_create(struct domain *d, ssidref_t ssidref)
{
    void *subject_ssid = current->domain->ssid;
    domid_t domid = d->domain_id;
    int rc;

    read_lock(&acm_bin_pol_rwlock);
    /*
       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(d, ssidref);
    if (rc != ACM_OK)
        goto error_out;

    if ((acm_primary_ops->domain_create != NULL) &&
        acm_primary_ops->domain_create(subject_ssid, ssidref, domid)) {
        rc = 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);
        rc = ACM_ACCESS_DENIED;
    }

    if ( rc == ACM_OK )
    {
        acm_domain_ssid_onto_list(d->ssid);
    } else {
        acm_free_domain_ssid(d->ssid);
    }

error_out:
    read_unlock(&acm_bin_pol_rwlock);
    return rc;
}


The acm_primary_ops->domain_create() establishes state (see 
chwall_domain_create() in acm_chinesewall_hooks.c ) that if the secondary 
operation fails needs to be undone. That's what the 
acm_primary_ops->domain_destroy() did, but you intend to remove it?! I 
have my doubts that this is correct.

Which NULL pointer is the code running into and where?

   Stefan


> 
> George
> 
> 
> On Thu, 2007-09-27 at 14:35 -0400, Stefan Berger wrote:
> > 
> > xense-devel-bounces@lists.xensource.com wrote on 09/27/2007 12:43:35
> > PM:
> > 
> > > The attached patch fixes a null dereference bug in XSM:ACM. 
> > 
> > As I read this in response to recent error reports - I wonder why CS
> > 15661 does not expose this error whereas afterwards this error occurs.
> > Are you sure this is the right solution? Was something changed in this
> > area of the code between 'before XSM' and afterwards? 
> > 
> >    Stefan 
> > 
> > 
> > 
> > > 
> > > Signed-off-by: George Coker <gscoker@alpha.ncsc.mil>
> > > [attachment "acm-xsm-null_bug-092707-xen-unstable-15880.diff" 
> > > deleted by Stefan Berger/Watson/IBM] 
> > > _______________________________________________
> > > Xense-devel mailing list
> > > Xense-devel@lists.xensource.com
> > > http://lists.xensource.com/xense-devel
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xensource.com
> > http://lists.xensource.com/xen-devel
> -- 
> George S. Coker, II <gscoker@alpha.ncsc.mil> 443-479-6944

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

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

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

Re: Re: [Xense-devel] [XSM:ACM][PATCH] nulldereference bug fix

[George S. Coker, II]

On Thu, 2007-09-27 at 16:12 -0400, Stefan Berger wrote:
> 
> "Coker, George" <gscoker@tycho.ncsc.mil> wrote on 09/27/2007 03:35:14
> PM:
> 
> > This patch is correct for XSM.  The patch creates clean
> > acm_domain_create and acm_domain_destroy operations.
> > 
> > In 15661 the logic under which acm_domain_destroy is called is
> slightly
> > different than under XSM.  In 15661, acm_domain_destroy is called
> only
> > if the mask INIT_acm is set.  INIT_acm is set only on successful
> return
> > from acm_domain_create.  When acm_domain_create fails, the mask is
> not
> > set and acm_domain_destroy is not called.  I do not know if this
> > resulted in a leak in 15661 due to incomplete cleanup. 
> 
> So the roll-back call that was necessary before is not necessary
> anymore? 
> 

Yes, because on fail, acm_domain_create will always return
ACM_ACCESS_DENIED which will cause domain_create to goto fail on return
from xsm_domain_create.  At fail, free_domain is called.  free_domain
calls xsm_free_security_domain which calls acm_domain_destroy.

acm_domain_destroy calls the acm_primary_ops->domain_destroy followed by
the acm_secondary_ops->domain_destroy.  acm_domain_destroy ends with
acm_free_domain_ssid.  acm_domain_destroy already contains all of the
rollback code that is replicated in acm_domain_create.

> static inline int acm_domain_create(struct domain *d, ssidref_t
> ssidref)
> {
>    void *subject_ssid = current->domain->ssid;
>    domid_t domid = d->domain_id;
>    int rc;
> 
>    read_lock(&acm_bin_pol_rwlock);
>    /*
>       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(d, ssidref);
>    if (rc != ACM_OK)
>        goto error_out;
> 
>    if ((acm_primary_ops->domain_create != NULL) &&
>        acm_primary_ops->domain_create(subject_ssid, ssidref, domid)) {
>        rc = 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);
>        rc = ACM_ACCESS_DENIED;
>    }
> 
>    if ( rc == ACM_OK )
>    {
>        acm_domain_ssid_onto_list(d->ssid);
>    } else {
>        acm_free_domain_ssid(d->ssid);
>    }
> 
> error_out:
>    read_unlock(&acm_bin_pol_rwlock);
>    return rc;
> } 
> 
> 
> The acm_primary_ops->domain_create() establishes state (see
> chwall_domain_create() in acm_chinesewall_hooks.c ) that if the
> secondary operation fails needs to be undone. That's what the
> acm_primary_ops->domain_destroy() did, but you intend to remove it?! I
> have my doubts that this is correct. 
> 
Yes I am removing the rollback from acm_domain_create because it is
already duplicated in acm_domain_destroy.  acm_domain_destroy is always
now called on fail under XSM.  In 15661, acm_domain_destroy was not
always called.

> Which NULL pointer is the code running into and where? 

The NULL pointer was created in acm_free_domain_ssid and dereferenced in
the fail code path in the call to xsm_free_security_domain in
free_domain.

> 
>    Stefan 
> 
> 
> > 
> > George
> > 
> > 
> > On Thu, 2007-09-27 at 14:35 -0400, Stefan Berger wrote:
> > > 
> > > xense-devel-bounces@lists.xensource.com wrote on 09/27/2007
> 12:43:35
> > > PM:
> > > 
> > > > The attached patch fixes a null dereference bug in XSM:ACM. 
> > > 
> > > As I read this in response to recent error reports - I wonder why
> CS
> > > 15661 does not expose this error whereas afterwards this error
> occurs.
> > > Are you sure this is the right solution? Was something changed in
> this
> > > area of the code between 'before XSM' and afterwards? 
> > > 
> > >    Stefan 
> > > 
> > > 
> > > 
> > > > 
> > > > Signed-off-by: George Coker <gscoker@alpha.ncsc.mil>
> > > > [attachment "acm-xsm-null_bug-092707-xen-unstable-15880.diff" 
> > > > deleted by Stefan Berger/Watson/IBM] 
> > > > _______________________________________________
> > > > Xense-devel mailing list
> > > > Xense-devel@lists.xensource.com
> > > > http://lists.xensource.com/xense-devel
> > > _______________________________________________
> > > Xen-devel mailing list
> > > Xen-devel@lists.xensource.com
> > > http://lists.xensource.com/xen-devel
> > -- 
> > George S. Coker, II <gscoker@alpha.ncsc.mil> 443-479-6944
> _______________________________________________
> Xense-devel mailing list
> Xense-devel@lists.xensource.com
> http://lists.xensource.com/xense-devel
-- 
George S. Coker, II <gscoker@alpha.ncsc.mil> 443-479-6944

Re: Re: [Xense-devel] [XSM:ACM][PATCH] nulldereference bug fix

[Syunsuke HAYASHI]

Hi,

I tested your patch.
I confirmed that XSM/ACM run normally.
Thank you for your help.

The result is put as follows.

#xm create vm1.conf <---- create domain1
Using config file "./vm1.conf".

#xm list --label
Name    ID Mem VCPUs  State   Time(s) Label
vm1      3 128  1 r-----  1.0 ACM:example.client_v1:dom_HomeBanking
Domain-0 0 743  2 r-----  0.0 ACM:example.client_v1:dom_SystemManagement

#xm create vm2.conf <---- create domain2
Using config file "./vm2.conf".
Internal error: Domain in conflict set with running domain?.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
#xm list --label
Name    ID Mem VCPUs  State   Time(s) Label
vm1      3 128  1 r-----  1.0 ACM:example.client_v1:dom_HomeBanking
Domain-0 0 743  2 r-----  0.0 ACM:example.client_v1:dom_SystemManagement


Chinese Wall looks good and "system boot" did not appen.


Syunsuke HAYASHI.

> On Thu, 2007-09-27 at 16:12 -0400, Stefan Berger wrote:
>> "Coker, George" <gscoker@tycho.ncsc.mil> wrote on 09/27/2007 03:35:14
>> PM:
>>
>>> This patch is correct for XSM.  The patch creates clean
>>> acm_domain_create and acm_domain_destroy operations.
>>>
>>> In 15661 the logic under which acm_domain_destroy is called is
>> slightly
>>> different than under XSM.  In 15661, acm_domain_destroy is called
>> only
>>> if the mask INIT_acm is set.  INIT_acm is set only on successful
>> return
>>> from acm_domain_create.  When acm_domain_create fails, the mask is
>> not
>>> set and acm_domain_destroy is not called.  I do not know if this
>>> resulted in a leak in 15661 due to incomplete cleanup. 
>> So the roll-back call that was necessary before is not necessary
>> anymore? 
>>
> 
> Yes, because on fail, acm_domain_create will always return
> ACM_ACCESS_DENIED which will cause domain_create to goto fail on return
> from xsm_domain_create.  At fail, free_domain is called.  free_domain
> calls xsm_free_security_domain which calls acm_domain_destroy.
> 
> acm_domain_destroy calls the acm_primary_ops->domain_destroy followed by
> the acm_secondary_ops->domain_destroy.  acm_domain_destroy ends with
> acm_free_domain_ssid.  acm_domain_destroy already contains all of the
> rollback code that is replicated in acm_domain_create.
> 
>> static inline int acm_domain_create(struct domain *d, ssidref_t
>> ssidref)
>> {
>>    void *subject_ssid = current->domain->ssid;
>>    domid_t domid = d->domain_id;
>>    int rc;
>>
>>    read_lock(&acm_bin_pol_rwlock);
>>    /*
>>       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(d, ssidref);
>>    if (rc != ACM_OK)
>>        goto error_out;
>>
>>    if ((acm_primary_ops->domain_create != NULL) &&
>>        acm_primary_ops->domain_create(subject_ssid, ssidref, domid)) {
>>        rc = 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);
>>        rc = ACM_ACCESS_DENIED;
>>    }
>>
>>    if ( rc == ACM_OK )
>>    {
>>        acm_domain_ssid_onto_list(d->ssid);
>>    } else {
>>        acm_free_domain_ssid(d->ssid);
>>    }
>>
>> error_out:
>>    read_unlock(&acm_bin_pol_rwlock);
>>    return rc;
>> } 
>>
>>
>> The acm_primary_ops->domain_create() establishes state (see
>> chwall_domain_create() in acm_chinesewall_hooks.c ) that if the
>> secondary operation fails needs to be undone. That's what the
>> acm_primary_ops->domain_destroy() did, but you intend to remove it?! I
>> have my doubts that this is correct. 
>>
> Yes I am removing the rollback from acm_domain_create because it is
> already duplicated in acm_domain_destroy.  acm_domain_destroy is always
> now called on fail under XSM.  In 15661, acm_domain_destroy was not
> always called.
> 
>> Which NULL pointer is the code running into and where? 
> 
> The NULL pointer was created in acm_free_domain_ssid and dereferenced in
> the fail code path in the call to xsm_free_security_domain in
> free_domain.
> 
>>    Stefan 
>>
>>
>>> George
>>>
>>>
>>> On Thu, 2007-09-27 at 14:35 -0400, Stefan Berger wrote:
>>>> xense-devel-bounces@lists.xensource.com wrote on 09/27/2007
>> 12:43:35
>>>> PM:
>>>>
>>>>> The attached patch fixes a null dereference bug in XSM:ACM. 
>>>> As I read this in response to recent error reports - I wonder why
>> CS
>>>> 15661 does not expose this error whereas afterwards this error
>> occurs.
>>>> Are you sure this is the right solution? Was something changed in
>> this
>>>> area of the code between 'before XSM' and afterwards? 
>>>>
>>>>    Stefan 
>>>>
>>>>
>>>>
>>>>> Signed-off-by: George Coker <gscoker@alpha.ncsc.mil>
>>>>> [attachment "acm-xsm-null_bug-092707-xen-unstable-15880.diff" 
>>>>> deleted by Stefan Berger/Watson/IBM] 
>>>>> _______________________________________________
>>>>> Xense-devel mailing list
>>>>> Xense-devel@lists.xensource.com
>>>>> http://lists.xensource.com/xense-devel
>>>> _______________________________________________
>>>> Xen-devel mailing list
>>>> Xen-devel@lists.xensource.com
>>>> http://lists.xensource.com/xen-devel
>>> -- 
>>> George S. Coker, II <gscoker@alpha.ncsc.mil> 443-479-6944
>> _______________________________________________
>> Xense-devel mailing list
>> Xense-devel@lists.xensource.com
>> http://lists.xensource.com/xense-devel


-- 
/////////////////////////////////////////////
富士通株式会社　新横浜TECHビル A館6F
サーバシステム事業本部　Linux技術開発統括部
Name   : Syunsuke HAYASHI (林 俊介)
TEL    : 7124-3846(内線) 045-473-9478(外線)
E-mail : syunsuke@jp.fujitsu.com
////////////////////////////////////////////

Re: Re: [Xense-devel] [XSM:ACM][PATCH] nulldereference bug fix

[Stefan Berger]

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

xen-devel-bounces@lists.xensource.com wrote on 09/27/2007 04:48:14 PM:

> On Thu, 2007-09-27 at 16:12 -0400, Stefan Berger wrote:
> > 
> > "Coker, George" <gscoker@tycho.ncsc.mil> wrote on 09/27/2007 03:35:14
> > PM:
> > 
> > > This patch is correct for XSM.  The patch creates clean
> > > acm_domain_create and acm_domain_destroy operations.
> > > 
> > > In 15661 the logic under which acm_domain_destroy is called is
> > slightly
> > > different than under XSM.  In 15661, acm_domain_destroy is called
> > only
> > > if the mask INIT_acm is set.  INIT_acm is set only on successful
> > return
> > > from acm_domain_create.  When acm_domain_create fails, the mask is
> > not
> > > set and acm_domain_destroy is not called.  I do not know if this
> > > resulted in a leak in 15661 due to incomplete cleanup. 
> > 
> > So the roll-back call that was necessary before is not necessary
> > anymore? 
> > 
> 
> Yes, because on fail, acm_domain_create will always return
> ACM_ACCESS_DENIED which will cause domain_create to goto fail on return
> from xsm_domain_create.  At fail, free_domain is called.  free_domain
> calls xsm_free_security_domain which calls acm_domain_destroy.

The problem is that the roll-back related to chinese wall must *only* be 
done when the check on the chinese wall was successful and NOT when it was 
not successful. 
Following the test Syunsuke HAYASHI describes in

http://lists.xensource.com/archives/html/xen-devel/2007-09/msg00514.html


I get the following after creating the 1st domain when doing an 'xm 
dumppolicy':



Policy dump:
============
POLICY REFERENCE = example.client_v1.
PolicyVer = 8c000000.
XML Vers. = 1.24
Magic     = 1debc.
Len       = 178.
Primary   = CHINESE WALL (c=1, off=40).
Secondary = SIMPLE TYPE ENFORCEMENT (c=2, off=b8).


Chinese Wall policy:
====================
Policy version= ffffe849.
Max Types     = 4.
Max Ssidrefs  = 7.
Max ConfSets  = 1.
Ssidrefs Off  = 24.
Conflicts Off = 5c.
Runing T. Off = 64.
C. Agg. Off   = 6c.

SSID To CHWALL-Type matrix:

   ssidref 0:  00 00 00 00 
   ssidref 1:  00 00 00 01  <-- Domain-0
   ssidref 2:  00 01 00 00 
   ssidref 3:  01 00 00 00 
   ssidref 4:  00 00 01 00 
   ssidref 5:  00 00 00 01 
   ssidref 6:  00 00 00 01 

Confict Sets:

   c-set 0:    01 00 01 00 

Running
Types:         00 00 01 01 

Conflict
Aggregate Set: 01 00 00 00 


Simple Type Enforcement policy:
===============================
Policy version= cef6c202.
Max Types     = 6.
Max Ssidrefs  = e.
Ssidrefs Off  = 14.

SSID To STE-Type matrix:

   ssidref 0: 00 00 00 00 00 00 
   ssidref 1: 01 01 01 01 01 01  <-- Domain-0
   ssidref 2: 01 00 00 00 00 00 
   ssidref 3: 00 01 00 00 00 00 
   ssidref 4: 00 00 00 00 01 00 
   ssidref 5: 01 01 01 00 01 00 
   ssidref 6: 00 01 00 01 01 00 
   ssidref 7: 00 00 01 00 00 00 
   ssidref 8: 00 00 00 00 00 01 
   ssidref 9: 00 00 00 01 00 00 
   ssidref a: 00 00 00 00 01 00 
   ssidref b: 00 01 00 00 00 00 
   ssidref c: 00 00 00 00 00 01 
   ssidref d: 00 00 00 00 01 00 




This is output is correct.


After trying to start the 2nd domain I now get:



Policy dump:
============
POLICY REFERENCE = example.client_v1.
PolicyVer = 0.
XML Vers. = 1.24
Magic     = 1debc.
Len       = 178.
Primary   = CHINESE WALL (c=1, off=40).
Secondary = SIMPLE TYPE ENFORCEMENT (c=2, off=b8).


Chinese Wall policy:
====================
Policy version= 100ff00.
Max Types     = 4.
Max Ssidrefs  = 7.
Max ConfSets  = 1.
Ssidrefs Off  = 24.
Conflicts Off = 5c.
Runing T. Off = 64.
C. Agg. Off   = 6c.

SSID To CHWALL-Type matrix:

   ssidref 0:  00 00 00 00 
   ssidref 1:  00 00 00 01  <-- Domain-0
   ssidref 2:  00 01 00 00 
   ssidref 3:  01 00 00 00 
   ssidref 4:  00 00 01 00 
   ssidref 5:  00 00 00 01 
   ssidref 6:  00 00 00 01 

Confict Sets:

   c-set 0:    01 00 01 00 

Running
Types:         ffff 00 01 01 

Conflict
Aggregate Set: 01 00 ffff 00 


Simple Type Enforcement policy:
===============================
Policy version= 0.
Max Types     = 6.
Max Ssidrefs  = e.
Ssidrefs Off  = 14.

SSID To STE-Type matrix:

   ssidref 0: 00 00 00 00 00 00 
   ssidref 1: 01 01 01 01 01 01  <-- Domain-0
   ssidref 2: 01 00 00 00 00 00 
   ssidref 3: 00 01 00 00 00 00 
   ssidref 4: 00 00 00 00 01 00 
   ssidref 5: 01 01 01 00 01 00 
   ssidref 6: 00 01 00 01 01 00 
   ssidref 7: 00 00 01 00 00 00 
   ssidref 8: 00 00 00 00 00 01 
   ssidref 9: 00 00 00 01 00 00 
   ssidref a: 00 00 00 00 01 00 
   ssidref b: 00 01 00 00 00 00 
   ssidref c: 00 00 00 00 00 01 
   ssidref d: 00 00 00 00 01 00 



Obviously the 'Running Types' and 'Conflict Aggregate Set' are showing 
wrong numbers due to the state on the chinese wall having been rolled 
back, although it should not have been. Also, the reason why this 
operation was protected through the surrounding lock is that while this 
test happens no policy change may occur, which would recalculate all the 
state. So I'd rather have this unrolling left where it was.

   Stefan





> 
> acm_domain_destroy calls the acm_primary_ops->domain_destroy followed by
> the acm_secondary_ops->domain_destroy.  acm_domain_destroy ends with
> acm_free_domain_ssid.  acm_domain_destroy already contains all of the
> rollback code that is replicated in acm_domain_create.
> 
> > static inline int acm_domain_create(struct domain *d, ssidref_t
> > ssidref)
> > {
> >    void *subject_ssid = current->domain->ssid;
> >    domid_t domid = d->domain_id;
> >    int rc;
> > 
> >    read_lock(&acm_bin_pol_rwlock);
> >    /*
> >       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(d, ssidref);
> >    if (rc != ACM_OK)
> >        goto error_out;
> > 
> >    if ((acm_primary_ops->domain_create != NULL) &&
> >        acm_primary_ops->domain_create(subject_ssid, ssidref, domid)) {
> >        rc = 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);
> >        rc = ACM_ACCESS_DENIED;
> >    }
> > 
> >    if ( rc == ACM_OK )
> >    {
> >        acm_domain_ssid_onto_list(d->ssid);
> >    } else {
> >        acm_free_domain_ssid(d->ssid);
> >    }
> > 
> > error_out:
> >    read_unlock(&acm_bin_pol_rwlock);
> >    return rc;
> > } 
> > 
> > 
> > The acm_primary_ops->domain_create() establishes state (see
> > chwall_domain_create() in acm_chinesewall_hooks.c ) that if the
> > secondary operation fails needs to be undone. That's what the
> > acm_primary_ops->domain_destroy() did, but you intend to remove it?! I
> > have my doubts that this is correct. 
> > 
> Yes I am removing the rollback from acm_domain_create because it is
> already duplicated in acm_domain_destroy.  acm_domain_destroy is always
> now called on fail under XSM.  In 15661, acm_domain_destroy was not
> always called.
> 
> > Which NULL pointer is the code running into and where? 
> 
> The NULL pointer was created in acm_free_domain_ssid and dereferenced in
> the fail code path in the call to xsm_free_security_domain in
> free_domain.
> 
> > 
> >    Stefan 
> > 
> > 
> > > 
> > > George
> > > 
> > > 
> > > On Thu, 2007-09-27 at 14:35 -0400, Stefan Berger wrote:
> > > > 
> > > > xense-devel-bounces@lists.xensource.com wrote on 09/27/2007
> > 12:43:35
> > > > PM:
> > > > 
> > > > > The attached patch fixes a null dereference bug in XSM:ACM. 
> > > > 
> > > > As I read this in response to recent error reports - I wonder why
> > CS
> > > > 15661 does not expose this error whereas afterwards this error
> > occurs.
> > > > Are you sure this is the right solution? Was something changed in
> > this
> > > > area of the code between 'before XSM' and afterwards? 
> > > > 
> > > >    Stefan 
> > > > 
> > > > 
> > > > 
> > > > > 
> > > > > Signed-off-by: George Coker <gscoker@alpha.ncsc.mil>
> > > > > [attachment "acm-xsm-null_bug-092707-xen-unstable-15880.diff" 
> > > > > deleted by Stefan Berger/Watson/IBM] 
> > > > > _______________________________________________
> > > > > Xense-devel mailing list
> > > > > Xense-devel@lists.xensource.com
> > > > > http://lists.xensource.com/xense-devel
> > > > _______________________________________________
> > > > Xen-devel mailing list
> > > > Xen-devel@lists.xensource.com
> > > > http://lists.xensource.com/xen-devel
> > > -- 
> > > George S. Coker, II <gscoker@alpha.ncsc.mil> 443-479-6944
> > _______________________________________________
> > Xense-devel mailing list
> > Xense-devel@lists.xensource.com
> > http://lists.xensource.com/xense-devel
> -- 
> George S. Coker, II <gscoker@alpha.ncsc.mil> 443-479-6944
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

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

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

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

Re: Re: [Xense-devel] [XSM:ACM][PATCH] nulldereference bug fix

[George S. Coker, II]

On Fri, 2007-09-28 at 10:21 -0400, Stefan Berger wrote:

> The problem is that the roll-back related to chinese wall must *only*
> be done when the check on the chinese wall was successful and NOT when
> it was not successful. 
> Following the test Syunsuke HAYASHI describes in 
> 
> http://lists.xensource.com/archives/html/xen-devel/2007-09/msg00514.html 
> 
> 
> I get the following after creating the 1st domain when doing an 'xm
> dumppolicy': 
> 
> 
> 
> Policy dump:
> ============
> POLICY REFERENCE = example.client_v1.
> PolicyVer = 8c000000.
> XML Vers. = 1.24
> Magic     = 1debc.
> Len       = 178.
> Primary   = CHINESE WALL (c=1, off=40).
> Secondary = SIMPLE TYPE ENFORCEMENT (c=2, off=b8).
> 
> 
> Chinese Wall policy:
> ====================
> Policy version= ffffe849.
> Max Types     = 4.
> Max Ssidrefs  = 7.
> Max ConfSets  = 1.
> Ssidrefs Off  = 24.
> Conflicts Off = 5c.
> Runing T. Off = 64.
> C. Agg. Off   = 6c.
> 
> SSID To CHWALL-Type matrix:
> 
>   ssidref 0:  00 00 00 00 
>   ssidref 1:  00 00 00 01  <-- Domain-0
>   ssidref 2:  00 01 00 00 
>   ssidref 3:  01 00 00 00 
>   ssidref 4:  00 00 01 00 
>   ssidref 5:  00 00 00 01 
>   ssidref 6:  00 00 00 01 
> 
> Confict Sets:
> 
>   c-set 0:    01 00 01 00 
> 
> Running
> Types:         00 00 01 01 
> 
> Conflict
> Aggregate Set: 01 00 00 00 
> 
> 
> Simple Type Enforcement policy:
> ===============================
> Policy version= cef6c202.
> Max Types     = 6.
> Max Ssidrefs  = e.
> Ssidrefs Off  = 14.
> 
> SSID To STE-Type matrix:
> 
>   ssidref 0: 00 00 00 00 00 00 
>   ssidref 1: 01 01 01 01 01 01  <-- Domain-0
>   ssidref 2: 01 00 00 00 00 00 
>   ssidref 3: 00 01 00 00 00 00 
>   ssidref 4: 00 00 00 00 01 00 
>   ssidref 5: 01 01 01 00 01 00 
>   ssidref 6: 00 01 00 01 01 00 
>   ssidref 7: 00 00 01 00 00 00 
>   ssidref 8: 00 00 00 00 00 01 
>   ssidref 9: 00 00 00 01 00 00 
>   ssidref a: 00 00 00 00 01 00 
>   ssidref b: 00 01 00 00 00 00 
>   ssidref c: 00 00 00 00 00 01 
>   ssidref d: 00 00 00 00 01 00 
> 
> 
> 
> 
> This is output is correct. 
> 
> 
> After trying to start the 2nd domain I now get: 
> 
> 
> 
> Policy dump:
> ============
> POLICY REFERENCE = example.client_v1.
> PolicyVer = 0.
> XML Vers. = 1.24
> Magic     = 1debc.
> Len       = 178.
> Primary   = CHINESE WALL (c=1, off=40).
> Secondary = SIMPLE TYPE ENFORCEMENT (c=2, off=b8).
> 
> 
> Chinese Wall policy:
> ====================
> Policy version= 100ff00.
> Max Types     = 4.
> Max Ssidrefs  = 7.
> Max ConfSets  = 1.
> Ssidrefs Off  = 24.
> Conflicts Off = 5c.
> Runing T. Off = 64.
> C. Agg. Off   = 6c.
> 
> SSID To CHWALL-Type matrix:
> 
>   ssidref 0:  00 00 00 00 
>   ssidref 1:  00 00 00 01  <-- Domain-0
>   ssidref 2:  00 01 00 00 
>   ssidref 3:  01 00 00 00 
>   ssidref 4:  00 00 01 00 
>   ssidref 5:  00 00 00 01 
>   ssidref 6:  00 00 00 01 
> 
> Confict Sets:
> 
>   c-set 0:    01 00 01 00 
> 
> Running
> Types:         ffff 00 01 01 
> 
> Conflict
> Aggregate Set: 01 00 ffff 00 
> 
> 
> Simple Type Enforcement policy:
> ===============================
> Policy version= 0.
> Max Types     = 6.
> Max Ssidrefs  = e.
> Ssidrefs Off  = 14.
> 
> SSID To STE-Type matrix:
> 
>   ssidref 0: 00 00 00 00 00 00 
>   ssidref 1: 01 01 01 01 01 01  <-- Domain-0
>   ssidref 2: 01 00 00 00 00 00 
>   ssidref 3: 00 01 00 00 00 00 
>   ssidref 4: 00 00 00 00 01 00 
>   ssidref 5: 01 01 01 00 01 00 
>   ssidref 6: 00 01 00 01 01 00 
>   ssidref 7: 00 00 01 00 00 00 
>   ssidref 8: 00 00 00 00 00 01 
>   ssidref 9: 00 00 00 01 00 00 
>   ssidref a: 00 00 00 00 01 00 
>   ssidref b: 00 01 00 00 00 00 
>   ssidref c: 00 00 00 00 00 01 
>   ssidref d: 00 00 00 00 01 00 
> 
> 
> 
> Obviously the 'Running Types' and 'Conflict Aggregate Set' are showing
> wrong numbers due to the state on the chinese wall having been rolled
> back, although it should not have been. Also, the reason why this
> operation was protected through the surrounding lock is that while
> this test happens no policy change may occur, which would recalculate
> all the state. So I'd rather have this unrolling left where it was. 
> 

> > 
> > > static inline int acm_domain_create(struct domain *d, ssidref_t
> > > ssidref)
> > > {
> > >    void *subject_ssid = current->domain->ssid;
> > >    domid_t domid = d->domain_id;
> > >    int rc;
> > > 
> > >    read_lock(&acm_bin_pol_rwlock);
> > >    /*
> > >       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(d, ssidref);
> > >    if (rc != ACM_OK)
> > >        goto error_out;
> > > 
> > >    if ((acm_primary_ops->domain_create != NULL) &&
> > >        acm_primary_ops->domain_create(subject_ssid, ssidref,
> domid)) {
> > >        rc = 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);
> > >        rc = ACM_ACCESS_DENIED;
> > >    }
> > > 
> > >    if ( rc == ACM_OK )
> > >    {
> > >        acm_domain_ssid_onto_list(d->ssid);
> > >    } else {
> > >        acm_free_domain_ssid(d->ssid);
> > >    }
> > > 
> > > error_out:
> > >    read_unlock(&acm_bin_pol_rwlock);
> > >    return rc;
> > > } 
> > > 

Ok, but then the null dereference issue still exists within the context
of XSM.  The options are:

1) Follow acm_free_domain_ssid(d->ssid) with d->ssid = NULL
(I've tested this and it maintains the desired policy state:

Confict Sets:

   c-set 0:    00 01 00 01

Running
Types:         01 01 00 00

Conflict
Aggregate Set: 00 00 00 01

but seems less attractive from a coding style perspective)

or 

2) Chance acm_free_domain_ssid to pass d instead of ssid and ensure that
d->ssid = NULL in acm_free_domain_ssid.

George

Re: Re: [Xense-devel] [XSM:ACM][PATCH] nulldereference bug fix

[Stefan Berger]

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

[...]
> > > > 
> 
> Ok, but then the null dereference issue still exists within the context
> of XSM.  The options are:
> 
> 1) Follow acm_free_domain_ssid(d->ssid) with d->ssid = NULL
> (I've tested this and it maintains the desired policy state:
> 
> Confict Sets:
> 
>    c-set 0:    00 01 00 01
> 
> Running
> Types:         01 01 00 00
> 
> Conflict
> Aggregate Set: 00 00 00 01
> 
> but seems less attractive from a coding style perspective)
> 
> or 
> 
> 2) Chance acm_free_domain_ssid to pass d instead of ssid and ensure that
> d->ssid = NULL in acm_free_domain_ssid.

Sounds like this is cleaner and so I opt for this choice.

   Stefan

> 
> George

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

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

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

Re: Re: [Xense-devel][XSM:ACM][PATCH] nulldereference bug fix

[Coker, George]

On Fri, 2007-09-28 at 11:20 -0400, Stefan Berger wrote:
> 
> [...]
> > > > > 
> > 
> > Ok, but then the null dereference issue still exists within the
> context
> > of XSM.  The options are:
> > 
> > 1) Follow acm_free_domain_ssid(d->ssid) with d->ssid = NULL
> > (I've tested this and it maintains the desired policy state:
> > 
> > Confict Sets:
> > 
> >    c-set 0:    00 01 00 01
> > 
> > Running
> > Types:         01 01 00 00
> > 
> > Conflict
> > Aggregate Set: 00 00 00 01
> > 
> > but seems less attractive from a coding style perspective)
> > 
> > or 
> > 
> > 2) Chance acm_free_domain_ssid to pass d instead of ssid and ensure
> that
> > d->ssid = NULL in acm_free_domain_ssid. 
> 
> Sounds like this is cleaner and so I opt for this choice. 
> 
I've just posted a patch.  Would you care to check it out and Ack it to
avoid any confusion?

George