BUG/MEDIUM: lists: Lock the element while we check if it is in a list.
authorOlivier Houchard <cognet@ci0.org>
Mon, 29 Jun 2020 17:52:01 +0000 (19:52 +0200)
committerOlivier Houchard <cognet@ci0.org>
Mon, 29 Jun 2020 17:59:06 +0000 (19:59 +0200)
In MT_LIST_ADDQ() and MT_LIST_ADD() we can't just check if the element is
already in a list, because there's a small race condition, it could be added
between the time we checked, and the time we actually set its next and prev.
So we have to lock it first.

This should be backported to 2.1.

include/haproxy/list.h

index 2f2699f..7f05be2 100644 (file)
         int _ret = 0;                                                      \
        struct mt_list *lh = (_lh), *el = (_el);                           \
        while (1) {                                                        \
-               struct mt_list *n;                                         \
-               struct mt_list *p;                                         \
+               struct mt_list *n, *n2;                                    \
+               struct mt_list *p, *p2;                                    \
                n = _HA_ATOMIC_XCHG(&(lh)->next, MT_LIST_BUSY);            \
                if (n == MT_LIST_BUSY)                                     \
                        continue;                                          \
                        __ha_barrier_store();                              \
                        continue;                                          \
                }                                                          \
-               if ((el)->next != (el) || (el)->prev != (el)) {            \
+               n2 = _HA_ATOMIC_XCHG(&(el)->next, MT_LIST_BUSY);           \
+               if (n2 != (el)) {                                          \
                        (n)->prev = p;                                     \
                        (lh)->next = n;                                    \
+                       (el)->next = n2;                                   \
+                       if (n2 == MT_LIST_BUSY)                            \
+                               continue;                                  \
+                       break;                                             \
+               }                                                          \
+               p2 = _HA_ATOMIC_XCHG(&(el)->prev, MT_LIST_BUSY);           \
+               if (p2 != (el)) {                                          \
+                       n->prev = p;                                       \
+                       (lh)->next = n;                                    \
+                       (el)->next = n2;                                   \
+                       (el)->prev = p2;                                   \
+                       if (p2 == MT_LIST_BUSY)                            \
+                               continue;                                  \
                        break;                                             \
                }                                                          \
                (el)->next = n;                                            \
        int _ret = 0;                                                      \
        struct mt_list *lh = (_lh), *el = (_el);                           \
        while (1) {                                                        \
-               struct mt_list *n;                                         \
-               struct mt_list *p;                                         \
+               struct mt_list *n, *n2;                                    \
+               struct mt_list *p, *p2;                                    \
                p = _HA_ATOMIC_XCHG(&(lh)->prev, MT_LIST_BUSY);            \
                if (p == MT_LIST_BUSY)                                     \
                        continue;                                          \
                        __ha_barrier_store();                              \
                        continue;                                          \
                }                                                          \
-               if ((el)->next != (el) || (el)->prev != (el)) {            \
+               n2 = _HA_ATOMIC_XCHG(&(el)->next, MT_LIST_BUSY);            \
+               if (n2 != (el)) {                                          \
                        p->next = n;                                       \
                        (lh)->prev = p;                                    \
+                       (el)->next = n2;                                   \
+                       if (n2 == MT_LIST_BUSY)                            \
+                               continue;                                  \
+                       break;                                             \
+               }                                                          \
+               p2 = _HA_ATOMIC_XCHG(&(el)->prev, MT_LIST_BUSY);            \
+               if (p2 != (el)) {                                          \
+                       p->next = n;                                       \
+                       (lh)->prev = p;                                    \
+                       (el)->next = n2;                                   \
+                       (el)->prev = p2;                                   \
+                       if (p2 == MT_LIST_BUSY)                            \
+                               continue;                                  \
                        break;                                             \
                }                                                          \
                (el)->next = n;                                            \