From 93b69d437effff11b1c37f330d3265c37ec2f84b Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Thu, 18 Oct 2012 14:53:58 -0700 Subject: [PATCH 1/2] Yama: add RCU to drop read locking Stop using spinlocks in the read path. Add RCU list to handle the readers. Signed-off-by: Kees Cook Reviewed-by: Serge E. Hallyn Acked-by: John Johansen --- security/yama/yama_lsm.c | 47 ++++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 23 deletions(-) diff --git a/security/yama/yama_lsm.c b/security/yama/yama_lsm.c index b4c29848b49d..70cd85e3ba30 100644 --- a/security/yama/yama_lsm.c +++ b/security/yama/yama_lsm.c @@ -30,6 +30,7 @@ struct ptrace_relation { struct task_struct *tracer; struct task_struct *tracee; struct list_head node; + struct rcu_head rcu; }; static LIST_HEAD(ptracer_relations); @@ -48,32 +49,31 @@ static DEFINE_SPINLOCK(ptracer_relations_lock); static int yama_ptracer_add(struct task_struct *tracer, struct task_struct *tracee) { - int rc = 0; - struct ptrace_relation *added; - struct ptrace_relation *entry, *relation = NULL; + struct ptrace_relation *relation, *added; added = kmalloc(sizeof(*added), GFP_KERNEL); if (!added) return -ENOMEM; + added->tracee = tracee; + added->tracer = tracer; + spin_lock_bh(&ptracer_relations_lock); - list_for_each_entry(entry, &ptracer_relations, node) - if (entry->tracee == tracee) { - relation = entry; - break; + rcu_read_lock(); + list_for_each_entry_rcu(relation, &ptracer_relations, node) { + if (relation->tracee == tracee) { + list_replace_rcu(&relation->node, &added->node); + kfree_rcu(relation, rcu); + goto out; } - if (!relation) { - relation = added; - relation->tracee = tracee; - list_add(&relation->node, &ptracer_relations); } - relation->tracer = tracer; + list_add_rcu(&added->node, &ptracer_relations); + +out: + rcu_read_unlock(); spin_unlock_bh(&ptracer_relations_lock); - if (added != relation) - kfree(added); - - return rc; + return 0; } /** @@ -84,15 +84,18 @@ static int yama_ptracer_add(struct task_struct *tracer, static void yama_ptracer_del(struct task_struct *tracer, struct task_struct *tracee) { - struct ptrace_relation *relation, *safe; + struct ptrace_relation *relation; spin_lock_bh(&ptracer_relations_lock); - list_for_each_entry_safe(relation, safe, &ptracer_relations, node) + rcu_read_lock(); + list_for_each_entry_rcu(relation, &ptracer_relations, node) { if (relation->tracee == tracee || (tracer && relation->tracer == tracer)) { - list_del(&relation->node); - kfree(relation); + list_del_rcu(&relation->node); + kfree_rcu(relation, rcu); } + } + rcu_read_unlock(); spin_unlock_bh(&ptracer_relations_lock); } @@ -217,11 +220,10 @@ static int ptracer_exception_found(struct task_struct *tracer, struct task_struct *parent = NULL; bool found = false; - spin_lock_bh(&ptracer_relations_lock); rcu_read_lock(); if (!thread_group_leader(tracee)) tracee = rcu_dereference(tracee->group_leader); - list_for_each_entry(relation, &ptracer_relations, node) + list_for_each_entry_rcu(relation, &ptracer_relations, node) if (relation->tracee == tracee) { parent = relation->tracer; found = true; @@ -231,7 +233,6 @@ static int ptracer_exception_found(struct task_struct *tracer, if (found && (parent == NULL || task_is_descendant(parent, tracer))) rc = 1; rcu_read_unlock(); - spin_unlock_bh(&ptracer_relations_lock); return rc; } From 235e752789eb65a81477bb82845323dfcbf93012 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Mon, 19 Nov 2012 15:21:26 -0800 Subject: [PATCH 2/2] Yama: remove locking from delete path Instead of locking the list during a delete, mark entries as invalid and trigger a workqueue to clean them up. This lets us easily handle task_free from interrupt context. Signed-off-by: Kees Cook --- security/yama/yama_lsm.c | 49 ++++++++++++++++++++++++++++++++++------ 1 file changed, 42 insertions(+), 7 deletions(-) diff --git a/security/yama/yama_lsm.c b/security/yama/yama_lsm.c index 70cd85e3ba30..2663145d1197 100644 --- a/security/yama/yama_lsm.c +++ b/security/yama/yama_lsm.c @@ -17,6 +17,7 @@ #include #include #include +#include #define YAMA_SCOPE_DISABLED 0 #define YAMA_SCOPE_RELATIONAL 1 @@ -29,6 +30,7 @@ static int ptrace_scope = YAMA_SCOPE_RELATIONAL; struct ptrace_relation { struct task_struct *tracer; struct task_struct *tracee; + bool invalid; struct list_head node; struct rcu_head rcu; }; @@ -36,6 +38,29 @@ struct ptrace_relation { static LIST_HEAD(ptracer_relations); static DEFINE_SPINLOCK(ptracer_relations_lock); +static void yama_relation_cleanup(struct work_struct *work); +static DECLARE_WORK(yama_relation_work, yama_relation_cleanup); + +/** + * yama_relation_cleanup - remove invalid entries from the relation list + * + */ +static void yama_relation_cleanup(struct work_struct *work) +{ + struct ptrace_relation *relation; + + spin_lock(&ptracer_relations_lock); + rcu_read_lock(); + list_for_each_entry_rcu(relation, &ptracer_relations, node) { + if (relation->invalid) { + list_del_rcu(&relation->node); + kfree_rcu(relation, rcu); + } + } + rcu_read_unlock(); + spin_unlock(&ptracer_relations_lock); +} + /** * yama_ptracer_add - add/replace an exception for this tracer/tracee pair * @tracer: the task_struct of the process doing the ptrace @@ -57,10 +82,13 @@ static int yama_ptracer_add(struct task_struct *tracer, added->tracee = tracee; added->tracer = tracer; + added->invalid = false; - spin_lock_bh(&ptracer_relations_lock); + spin_lock(&ptracer_relations_lock); rcu_read_lock(); list_for_each_entry_rcu(relation, &ptracer_relations, node) { + if (relation->invalid) + continue; if (relation->tracee == tracee) { list_replace_rcu(&relation->node, &added->node); kfree_rcu(relation, rcu); @@ -72,7 +100,7 @@ static int yama_ptracer_add(struct task_struct *tracer, out: rcu_read_unlock(); - spin_unlock_bh(&ptracer_relations_lock); + spin_unlock(&ptracer_relations_lock); return 0; } @@ -85,18 +113,22 @@ static void yama_ptracer_del(struct task_struct *tracer, struct task_struct *tracee) { struct ptrace_relation *relation; + bool marked = false; - spin_lock_bh(&ptracer_relations_lock); rcu_read_lock(); list_for_each_entry_rcu(relation, &ptracer_relations, node) { + if (relation->invalid) + continue; if (relation->tracee == tracee || (tracer && relation->tracer == tracer)) { - list_del_rcu(&relation->node); - kfree_rcu(relation, rcu); + relation->invalid = true; + marked = true; } } rcu_read_unlock(); - spin_unlock_bh(&ptracer_relations_lock); + + if (marked) + schedule_work(&yama_relation_work); } /** @@ -223,12 +255,15 @@ static int ptracer_exception_found(struct task_struct *tracer, rcu_read_lock(); if (!thread_group_leader(tracee)) tracee = rcu_dereference(tracee->group_leader); - list_for_each_entry_rcu(relation, &ptracer_relations, node) + list_for_each_entry_rcu(relation, &ptracer_relations, node) { + if (relation->invalid) + continue; if (relation->tracee == tracee) { parent = relation->tracer; found = true; break; } + } if (found && (parent == NULL || task_is_descendant(parent, tracer))) rc = 1;