Messages in this thread | | | From | "Hillf Danton" <> | Subject | Re: [PATCH v4 09/11] smack: namespace groundwork | Date | Fri, 16 Oct 2015 11:04:54 +0800 |
| |
> On czw, 2015-10-15 at 14:41 +0200, Lukasz Pawelczyk wrote: > > > No, not a typo. A regular bug. Thanks for spotting it. Also sync > > mechanism before freeing was missing: > > > Hitfix, will be integrated with the next respin: > > diff --git a/security/smack/smack.h b/security/smack/smack.h > index 3d432f4..3a795bf 100644 > --- a/security/smack/smack.h > +++ b/security/smack/smack.h > @@ -97,6 +97,7 @@ struct smack_ns { > struct smack_known_ns { > struct list_head smk_list_known; > struct list_head smk_list_ns; > + struct rcu_head smk_rcu; > struct user_namespace *smk_ns; > char *smk_mapped; > struct smack_known *smk_unmapped; > diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c > index 8e0da67..234da71 100644 > --- a/security/smack/smack_lsm.c > +++ b/security/smack/smack_lsm.c > @@ -4669,6 +4669,15 @@ static inline int smack_userns_create(struct user_namespace *ns) > return 0; > } > > +static void smk_free_known_ns(struct rcu_head *head) > +{ > + struct smack_known_ns *sknp = container_of(head, struct smack_known_ns, smk_rcu); > + > + if (sknp->smk_allocated) > + kfree(sknp->smk_mapped); > + kfree(sknp); > +} > + > static inline void smack_userns_free(struct user_namespace *ns) > { > struct smack_ns *snsp = ns->security; > @@ -4680,12 +4689,11 @@ static inline void smack_userns_free(struct user_namespace *ns) > > mutex_lock(&skp->smk_mapped_lock); > list_del_rcu(&sknp->smk_list_known); > - if (sknp->smk_allocated) > - kfree(sknp->smk_mapped); > - kfree(sknp); > mutex_unlock(&skp->smk_mapped_lock); > > list_del(&sknp->smk_list_ns);
Is list_del safe, given the operation
+ mutex_lock(&snsp->smk_mapped_lock); + list_add_rcu(&sknp->smk_list_ns, &snsp->smk_mapped); + mutex_unlock(&snsp->smk_mapped_lock);
in smk_import_mapped() function(copied below)?
> + > + call_rcu(&sknp->smk_rcu, smk_free_known_ns); > } > > kfree(snsp); > -- >
+static struct smack_known_ns *smk_import_mapped(struct smack_known *skp, + struct user_namespace *ns, + const char *string, int len) +{ + struct smack_ns *snsp = ns->security; + struct smack_known_ns *sknp; + char *mapped; + bool allocated; + + /* Mapping init_user_ns is against the design and pointless */ + if (ns == &init_user_ns) + return ERR_PTR(-EBADR); + + mapped = smk_parse_smack(string, len, &allocated); + if (IS_ERR(mapped)) + return ERR_CAST(mapped); + + mutex_lock(&skp->smk_mapped_lock); + + /* + * Don't allow one<->many mappings in namespace, rename. + * This code won't get triggered for now as trying to assign + * a duplicate is forbidden in proc_label_map_write(). + * Leaving this as this function might be also used elsewhere. + */ + sknp = smk_find_mapped(skp, ns); + if (sknp != NULL) { + if (sknp->smk_allocated) + kfree(sknp->smk_mapped); + sknp->smk_mapped = mapped; + sknp->smk_allocated = allocated; + goto unlockout; + } + + sknp = kzalloc(sizeof(*sknp), GFP_KERNEL); + if (sknp == NULL) { + sknp = ERR_PTR(-ENOMEM); + if (allocated) + kfree(mapped); + goto unlockout; + } + + sknp->smk_ns = ns; + sknp->smk_mapped = mapped; + sknp->smk_allocated = allocated; + sknp->smk_unmapped = skp; + list_add_rcu(&sknp->smk_list_known, &skp->smk_mapped); + + mutex_lock(&snsp->smk_mapped_lock); + list_add_rcu(&sknp->smk_list_ns, &snsp->smk_mapped); + mutex_unlock(&snsp->smk_mapped_lock); + +unlockout: + mutex_unlock(&skp->smk_mapped_lock); + + return sknp; +} > > > -- > Lukasz Pawelczyk > Samsung R&D Institute Poland > Samsung Electronics > >
| |