> 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
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
