Messages in this thread Patch in this message | | | Date | Tue, 26 Aug 2014 18:41:29 +0300 | From | Andreea-Cristina Bernat <> | Subject | [PATCH RFC] rcu: assoc_array: Add critical section to avoid a bug |
| |
* The function "assoc_array_apply_edit()" passes "edit->rcu" to "call_rcu()". * "assoc_array_apply_edit()'s" callers do not call it under an RCU read-side critical section. * The function "assoc_array_gc()" could be preempted between the call to "assoc_array_apply_edit()" call and the assignment "edit->array->nr_leaves_on_tree = nr_leaves_on_tree;", so the grace period could complete. * The problem is that the assignment could access the freelist. * Solution: Put the call to the "assoc_array_apply_edit()" function and the assignment between "rcu_read_lock()" and "rcu_read_unlock()". In this way, the callback function provided by the "call_rcu()" call will be invoked after all pre-existing critical sections end which means after the added critical section.
The first step to detect this was made with the following Coccinelle semantic patch which looks for calls of functions which have in their bodies a call to "call_rcu()" with a global first parameter:
@ locally @ identifier l; type t; position p; @@
t l; ... when any call_rcu@p((<+...l...+>), ...);
@ globally @ identifier fn; identifier g != locally.l; position p2 != locally.p; @@
fn(...) { ... when any call_rcu@p2((<+...g...+>), ...); ... when any }
@ other_func @ identifier globally.fn, ff; @@
ff(...) { ... when any * fn(...) ... when any }
The second step was looking at the uses of the functions found and see if the parameter of interest is used after the call of the function.
Signed-off-by: Andreea-Cristina Bernat <bernat.ada@gmail.com> --- lib/assoc_array.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/lib/assoc_array.c b/lib/assoc_array.c index c0b1007..45d11c0 100644 --- a/lib/assoc_array.c +++ b/lib/assoc_array.c @@ -1734,8 +1734,10 @@ ascend_old_tree: gc_complete: edit->set[0].to = new_root; + rcu_read_lock(); assoc_array_apply_edit(edit); edit->array->nr_leaves_on_tree = nr_leaves_on_tree; + rcu_read_unlock(); return 0; enomem: -- 1.9.1
| |