From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752831AbbJPDLO (ORCPT ); Thu, 15 Oct 2015 23:11:14 -0400 Received: from out4133-146.mail.aliyun.com ([42.120.133.146]:41640 "EHLO out4133-146.mail.aliyun.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751889AbbJPDLK (ORCPT ); Thu, 15 Oct 2015 23:11:10 -0400 X-Alimail-AntiSpam: AC=PASS;BC=-1|-1;BR=01201311R281e4;FP=0|-1|-1|-1|0|-1|-1|-1;HT=e02c03305;MF=hillf.zj@alibaba-inc.com;NM=1;PH=DS;RN=5;SR=0; Reply-To: "Hillf Danton" From: "Hillf Danton" To: "'Lukasz Pawelczyk'" Cc: "'Andy Lutomirski'" , "'Kees Cook'" , "'linux-kernel'" , References: <019801d1071b$2c124000$8436c000$@alibaba-inc.com> <01a101d1071c$8ce631b0$a6b29510$@alibaba-inc.com> <1444912906.5661.7.camel@samsung.com> <1444913602.5661.9.camel@samsung.com> In-Reply-To: <1444913602.5661.9.camel@samsung.com> Subject: Re: [PATCH v4 09/11] smack: namespace groundwork Date: Fri, 16 Oct 2015 11:04:54 +0800 Message-ID: <026701d107bf$6ef3e280$4cdba780$@alibaba-inc.com> MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit X-Mailer: Microsoft Outlook 14.0 Thread-Index: AQGwINKpDRsl9OAWOmdjA0WePMlYygIl2IFaAVOugGoBogoXq56GWpiw Content-Language: zh-cn Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > 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 > >