Re: [Xen-devel] Re: [Xense-devel] [XSM:ACM][PATCH] nulldereferen

xen-devel-bounces@xxxxxxxxxxxxxxxxxxx wrote on 09/27/2007 04:48:14 PM: > On Thu, 2007-09-27 at 16:12 -0400, Stefan Berger wrote: > > > > "Coker, George" <gscoker@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxx 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@xxxxxxxxxxxxxx> > > > > > [attachment "acm-xsm-null_bug-092707-xen-unstable-15880.diff" > > > > > deleted by Stefan Berger/Watson/IBM] > > > > > _______________________________________________ > > > > > Xense-devel mailing list > > > > > Xense-devel@xxxxxxxxxxxxxxxxxxx > > > > > http://lists.xensource.com/xense-devel > > > > _______________________________________________ > > > > Xen-devel mailing list > > > > Xen-devel@xxxxxxxxxxxxxxxxxxx > > > > http://lists.xensource.com/xen-devel > > > -- > > > George S. Coker, II <gscoker@xxxxxxxxxxxxxx> 443-479-6944 > > _______________________________________________ > > Xense-devel mailing list > > Xense-devel@xxxxxxxxxxxxxxxxxxx > > http://lists.xensource.com/xense-devel > -- > George S. Coker, II <gscoker@xxxxxxxxxxxxxx> 443-479-6944 > > _______________________________________________ > Xen-devel mailing list > Xen-devel@xxxxxxxxxxxxxxxxxxx > http://lists.xensource.com/xen-devel

_______________________________________________
Xense-devel mailing list
Xense-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xense-devel

WARNING - OLD ARCHIVES

xense-devel

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