linux-kernel - [PATCH RFC] rcu: assoc_array: Add critical section to avoid a bug

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 26 Aug 2014 18:41:29 +0300
From:	Andreea-Cristina Bernat <bernat.ada@...il.com>
To:	dhowells@...hat.com, shemming@...cade.com,
	linux-kernel@...r.kernel.org
Cc:	paulmck@...ux.vnet.ibm.com
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@...il.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

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