From 347bbf79d20e1cff57075a8a378355dfac2475e2 Mon Sep 17 00:00:00 2001 From: Olivier Houchard Date: Mon, 29 Jun 2020 19:52:01 +0200 Subject: [PATCH] BUG/MEDIUM: lists: Lock the element while we check if it is in a list. 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 | 40 ++++++++++++++++++++++++++++++++++------ 1 file changed, 34 insertions(+), 6 deletions(-) diff --git a/include/haproxy/list.h b/include/haproxy/list.h index 2f2699f17..7f05be2f9 100644 --- a/include/haproxy/list.h +++ b/include/haproxy/list.h @@ -230,8 +230,8 @@ 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; \ @@ -241,9 +241,23 @@ __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; \ @@ -269,8 +283,8 @@ 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; \ @@ -280,9 +294,23 @@ __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; \