Fix: handle reference count overflow

The urcu refcounting API features a look and feel similar to the Linux
kernel reference counting API, which has been the subject of
CVE-2016-0728 (use-after-free). Therefore, improve the urcu refcounting
API by dealing with reference counting overflow.

For urcu_ref_get(), handle this by comparing the prior value with
LONG_MAX before updating it with a cmpxchg. When an overflow would
occur, trigger a abort() rather than allowing the overflow (which is a
use-after-free security concern).

For urcu_ref_get_unless_zero(), in addition to compare the prior value
to 0, also compare it to LONG_MAX, and return failure (false) in both
cases.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

diff --git a/urcu/ref.h b/urcu/ref.h
index 74be50d6bbe4e46d28603ba4cb8de3db2ed7794a..2b803e5b68e4710606df29f128ff7f1637dc1472 100644 (file)
--- a/urcu/ref.h
+++ b/urcu/ref.h
@@ -16,6 +16,8 @@
 
 #include <assert.h>
 #include <stdbool.h>
+#include <limits.h>
+#include <stdlib.h>
 #include <urcu/uatomic.h>
 
 struct urcu_ref {
@@ -34,7 +36,20 @@ static inline void urcu_ref_init(struct urcu_ref *ref)
 
 static inline void urcu_ref_get(struct urcu_ref *ref)
 {
-       uatomic_add(&ref->refcount, 1);
+       long old, _new, res;
+
+       old = uatomic_read(&ref->refcount);
+       for (;;) {
+               if (old == LONG_MAX) {
+                       abort();
+               }
+               _new = old + 1;
+               res = uatomic_cmpxchg(&ref->refcount, old, _new);
+               if (res == old) {
+                       return;
+               }
+               old = res;
+       }
 }
 
 static inline void urcu_ref_put(struct urcu_ref *ref,
@@ -53,7 +68,8 @@ static inline void urcu_ref_put(struct urcu_ref *ref,
  * zero. Returns true if the reference is taken, false otherwise. This
  * needs to be used in conjunction with another synchronization
  * technique (e.g.  RCU or mutex) to ensure existence of the reference
- * count.
+ * count. False is also returned in case incrementing the refcount would
+ * result in an overflow.
  */
 static inline bool urcu_ref_get_unless_zero(struct urcu_ref *ref)
 {
@@ -61,7 +77,7 @@ static inline bool urcu_ref_get_unless_zero(struct urcu_ref *ref)
 
        old = uatomic_read(&ref->refcount);
        for (;;) {
-               if (old == 0)
+               if (old == 0 || old == LONG_MAX)
                        return false;   /* Failure. */
                _new = old + 1;
                res = uatomic_cmpxchg(&ref->refcount, old, _new);