Fix: hlist iteration relies on undefined behavior
authorMathieu Desnoyers <mathieu.desnoyers@efficios.com>
Tue, 20 Apr 2021 20:07:54 +0000 (16:07 -0400)
committerMathieu Desnoyers <mathieu.desnoyers@efficios.com>
Thu, 22 Apr 2021 12:33:00 +0000 (08:33 -0400)
Comparing an offset from an object with NULL is undefined behavior
and the compiler may assume that this is never true.

This is indeed what is observed with gcc-10 miscompiling
cds_hlist_for_each_entry_rcu_2().

Fix this by introducing cds_hlist_entry_safe() rather than open-coding
the NULL check comparisons, and move cds_hlist_for_each_entry_2()
and cds_hlist_for_each_entry_safe_2() to this scheme as well.

Fixes: #1308
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Change-Id: Ief3531cf04e54b6dae05eb28a6822adfa141fdeb

include/urcu/hlist.h
include/urcu/rcuhlist.h

index 344481133eac2051f4d7ab85e6188f51887e7dd7..5e3c27d02025bd4445efd65606f60374d8326e88 100644 (file)
@@ -17,6 +17,7 @@
  */
 
 #include <stddef.h>
+#include <urcu/compiler.h>
 
 struct cds_hlist_head {
        struct cds_hlist_node *next;
@@ -40,8 +41,14 @@ void CDS_INIT_HLIST_HEAD(struct cds_hlist_head *ptr)
        { .next = NULL }
 
 /* Get typed element from list at a given position. */
-#define cds_hlist_entry(ptr, type, member) \
-       ((type *) ((char *) (ptr) - (unsigned long) (&((type *) 0)->member)))
+#define cds_hlist_entry(ptr, type, member)     caa_container_of(ptr, type, member)
+
+/* Get typed element from list at a given position, keeping NULL pointers. */
+#define cds_hlist_entry_safe(ptr, type, member)                                        \
+       ({                                                                      \
+               __typeof__(ptr) ____ret = ptr;                                  \
+               ____ret ? cds_hlist_entry(____ret, type, member) : NULL;        \
+       })
 
 /* Add new element at the head of the list. */
 static inline
@@ -93,17 +100,13 @@ void cds_hlist_del(struct cds_hlist_node *elem)
                        entry = cds_hlist_entry(pos, __typeof__(*entry), member))
 
 #define cds_hlist_for_each_entry_2(entry, head, member) \
-       for (entry = ((head)->next == NULL ? NULL \
-                       : cds_hlist_entry((head)->next, __typeof__(*entry), member)); \
+       for (entry = cds_hlist_entry_safe((head)->next, __typeof__(*entry), member); \
                entry != NULL; \
-               entry = (entry->member.next == NULL ? NULL \
-                       : cds_hlist_entry(entry->member.next, __typeof__(*entry), member)))
+               entry = cds_hlist_entry_safe(entry->member.next, __typeof__(*entry), member))
 
 #define cds_hlist_for_each_entry_safe_2(entry, e, head, member) \
-       for (entry = ((head)->next == NULL ? NULL \
-                       : cds_hlist_entry((head)->next, __typeof__(*entry), member)); \
-               (entry != NULL) && (e = (entry->member.next == NULL ? NULL \
-                                       : cds_hlist_entry(entry->member.next, \
+       for (entry = cds_hlist_entry_safe((head)->next, __typeof__(*entry), member); \
+               (entry != NULL) && (e = (cds_hlist_entry_safe(entry->member.next, \
                                                __typeof__(*entry), member)), 1); \
                entry = e)
 
index 69c4d3184fd50d7f38a730e17447678f5fec3991..ca1da068b1fa6fdbe95db84dae922fc44e66bf84 100644 (file)
@@ -72,10 +72,10 @@ void cds_hlist_del_rcu(struct cds_hlist_node *elem)
                        entry = cds_hlist_entry(pos, __typeof__(*entry), member))
 
 #define cds_hlist_for_each_entry_rcu_2(entry, head, member) \
-       for (entry = cds_hlist_entry(rcu_dereference((head)->next), \
+       for (entry = cds_hlist_entry_safe(rcu_dereference((head)->next), \
                        __typeof__(*entry), member); \
-               &entry->member != NULL; \
-               entry = cds_hlist_entry(rcu_dereference(entry->member.next), \
+               entry != NULL; \
+               entry = cds_hlist_entry_safe(rcu_dereference(entry->member.next), \
                        __typeof__(*entry), member))
 
 #endif /* _URCU_RCUHLIST_H */
This page took 0.03649 seconds and 4 git commands to generate.