--- Al Viro <[EMAIL PROTECTED]> wrote: > On Tue, Oct 16, 2007 at 09:17:40PM -0700, Casey Schaufler wrote: > > At random: > > > +static int smack_netlabel(struct sock *sk) > > +{ > > + static int initialized; > > + struct socket_smack *ssp = sk->sk_security; > > + struct netlbl_lsm_secattr secattr; > > + int rc = 0; > > + > > + if (!initialized) { > > + smk_cipso_doi(); > > + initialized = 1; > > + } > > And just what happens if another task calls the same while we are > blocked on allocation in smk_cipso_doi()?
I assume that swapping the two lines, initialized = 1; smk_cipso_doi(); although it looks like it would suffice, isn't really good enough. > Another problem is your handling of smk_known - you add to head under > mutex; fine. However, you read without one _and_ have no barriers > in initializing new list entries. > > Think what happens if CPU1 adds to list and CPU2 sees write to smk_known > *before* it sees write to ->smk_next. We see a single-element list and > we'll be lucky if that single entry won't be FUBAR. Help me understand this: if (skp == NULL) { skp = kzalloc(sizeof(struct smack_known), GFP_KERNEL); if (skp != NULL) { skp->smk_next = smack_known; strncpy(skp->smk_known, smack, SMK_MAXLEN); skp->smk_secid = smack_next_secid++; skp->smk_cipso = NULL; spin_lock_init(&skp->smk_cipsolock); smack_known = skp; } } CPU1 sets smk_next to smack_known. CPU1 fills in the rest of the entry. CPU1 sets smack_known to skp (the entry). CPU2 will either see the old value for smack_known, in which case this entry isn't actually on the list yet, or it will see the new value in smack_known. Since smk_next is set before the entry is added to the list, it seems that the scenario you've outlined shouldn't happen. I assume then that you're refering to a case where the memory seen by the two CPUs doesn't match. That still wouldn't account for the "single entry list" notion. If CPU2 sees anything in smk_next it should be the old smack_known. Casey Schaufler [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html