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

[Stefan Berger <stefanb@xxxxxxxxxx>]

"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?

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@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
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel