From: Olivier Houchard Date: Wed, 25 Nov 2020 19:38:00 +0000 (+0100) Subject: BUG/MEDIUM: lists: Lock the element while we check if it is in a list. X-Git-Tag: v2.1.12~15 X-Git-Url: http://git.haproxy.org/?a=commitdiff_plain;h=ba90d530cf804d29f65d62e60242567031c9286d;p=haproxy-2.1.git BUG/MEDIUM: lists: Lock the element while we check if it is in a list. In MT_LIST_TRY_ADDQ() and MT_LIST_TRY_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 is required to address issue #958. This should be backported to 2.3, 2.2 and 2.1. (cherry picked from commit 1f05324cbe92a7dde71f44dc740eb8240539746f) Signed-off-by: Christopher Faulet (cherry picked from commit c4f3013d9452707d4efbda455360c8d2af022411) [wt: fix has been in 2.3 for 3 versions now] Signed-off-by: Willy Tarreau (cherry picked from commit 4aebb4e88ed360003efacc7f75a48c1d39bda6bb) [wt: updated the two macros to match the equivalent upstream patch because during the reorg they were cleaned up by trimming the unused do{}while(0), making the patch impossible to apply that way] Signed-off-by: Willy Tarreau --- diff --git a/include/common/mini-clist.h b/include/common/mini-clist.h index bdb9212..0bd5622 100644 --- a/include/common/mini-clist.h +++ b/include/common/mini-clist.h @@ -229,36 +229,53 @@ struct cond_wordlist { ({ \ int _ret = 0; \ struct mt_list *lh = (_lh), *el = (_el); \ - do { \ - while (1) { \ - struct mt_list *n; \ - struct mt_list *p; \ - n = _HA_ATOMIC_XCHG(&(lh)->next, MT_LIST_BUSY); \ - if (n == MT_LIST_BUSY) \ - continue; \ - p = _HA_ATOMIC_XCHG(&n->prev, MT_LIST_BUSY); \ - if (p == MT_LIST_BUSY) { \ - (lh)->next = n; \ - __ha_barrier_store(); \ - continue; \ - } \ - if ((el)->next != (el) || (el)->prev != (el)) { \ - (n)->prev = p; \ - (lh)->next = n; \ - __ha_barrier_store(); \ - break; \ - } \ - (el)->next = n; \ - (el)->prev = p; \ + while (1) { \ + 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; \ + p = _HA_ATOMIC_XCHG(&n->prev, MT_LIST_BUSY); \ + if (p == MT_LIST_BUSY) { \ + (lh)->next = n; \ __ha_barrier_store(); \ - n->prev = (el); \ + continue; \ + } \ + n2 = _HA_ATOMIC_XCHG(&el->next, MT_LIST_BUSY); \ + if (n2 != el) { /* element already linked */ \ + if (n2 != MT_LIST_BUSY) \ + el->next = n2; \ + n->prev = p; \ __ha_barrier_store(); \ - p->next = (el); \ + lh->next = n; \ __ha_barrier_store(); \ - _ret = 1; \ + if (n2 == MT_LIST_BUSY) \ + continue; \ break; \ } \ - } while (0); \ + p2 = _HA_ATOMIC_XCHG(&el->prev, MT_LIST_BUSY); \ + if (p2 != el) { \ + if (p2 != MT_LIST_BUSY) \ + el->prev = p2; \ + n->prev = p; \ + el->next = el; \ + __ha_barrier_store(); \ + lh->next = n; \ + __ha_barrier_store(); \ + if (p2 == MT_LIST_BUSY) \ + continue; \ + break; \ + } \ + (el)->next = n; \ + (el)->prev = p; \ + __ha_barrier_store(); \ + n->prev = (el); \ + __ha_barrier_store(); \ + p->next = (el); \ + __ha_barrier_store(); \ + _ret = 1; \ + break; \ + } \ (_ret); \ }) @@ -269,38 +286,55 @@ struct cond_wordlist { */ #define MT_LIST_ADDQ(_lh, _el) \ ({ \ - int _ret = 0; \ - struct mt_list *lh = (_lh), *el = (_el); \ - do { \ - while (1) { \ - struct mt_list *n; \ - struct mt_list *p; \ - p = _HA_ATOMIC_XCHG(&(lh)->prev, MT_LIST_BUSY); \ - if (p == MT_LIST_BUSY) \ - continue; \ - n = _HA_ATOMIC_XCHG(&p->next, MT_LIST_BUSY); \ - if (n == MT_LIST_BUSY) { \ - (lh)->prev = p; \ - __ha_barrier_store(); \ - continue; \ - } \ - if ((el)->next != (el) || (el)->prev != (el)) { \ - p->next = n; \ - (lh)->prev = p; \ - __ha_barrier_store(); \ - break; \ - } \ - (el)->next = n; \ - (el)->prev = p; \ + int _ret = 0; \ + struct mt_list *lh = (_lh), *el = (_el); \ + while (1) { \ + 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; \ + n = _HA_ATOMIC_XCHG(&p->next, MT_LIST_BUSY); \ + if (n == MT_LIST_BUSY) { \ + (lh)->prev = p; \ __ha_barrier_store(); \ - p->next = (el); \ + continue; \ + } \ + n2 = _HA_ATOMIC_XCHG(&el->next, MT_LIST_BUSY); \ + if (n2 != el) { /* element already linked */ \ + if (n2 != MT_LIST_BUSY) \ + el->next = n2; \ + p->next = n; \ __ha_barrier_store(); \ - n->prev = (el); \ + lh->prev = p; \ __ha_barrier_store(); \ - _ret = 1; \ + if (n2 == MT_LIST_BUSY) \ + continue; \ break; \ } \ - } while (0); \ + p2 = _HA_ATOMIC_XCHG(&el->prev, MT_LIST_BUSY); \ + if (p2 != el) { \ + if (p2 != MT_LIST_BUSY) \ + el->prev = p2; \ + p->next = n; \ + el->next = el; \ + __ha_barrier_store(); \ + lh->prev = p; \ + __ha_barrier_store(); \ + if (p2 == MT_LIST_BUSY) \ + continue; \ + break; \ + } \ + (el)->next = n; \ + (el)->prev = p; \ + __ha_barrier_store(); \ + p->next = (el); \ + __ha_barrier_store(); \ + n->prev = (el); \ + __ha_barrier_store(); \ + _ret = 1; \ + break; \ + } \ (_ret); \ })