BUG/MEDIUM: stick-tables: close a tiny race in __stksess_kill()

It might be possible not to see the element in the tree, then not to
see it in the update list, thus not to take the lock before deleting.
But an element in the list could have moved to the tree during the
check, and be removed later without the updt_lock.

Let's delete prior to checking the presence in the tree to avoid
this situation. No backport is needed since this arrived in -dev13
with the update list.

diff --git a/src/stick_table.c b/src/stick_table.c
index bf6ae39898863d80b83e479b615a404f14ff5366..26747eac3d1d5b5001edbd17ff00b72934bee888 100644 (file)
--- a/src/stick_table.c
+++ b/src/stick_table.c
@@ -144,13 +144,20 @@ int __stksess_kill(struct stktable *t, struct stksess *ts)
        if (HA_ATOMIC_LOAD(&ts->ref_cnt))
                return 0;
 
-       if (ts->upd.node.leaf_p || !MT_LIST_ISEMPTY(&ts->pend_updts)) {
+       /* make sure we're no longer in the updates list */
+       MT_LIST_DELETE(&ts->pend_updts);
+
+       /* ... and that nobody added us in between */
+       if (HA_ATOMIC_LOAD(&ts->ref_cnt))
+               return 0;
+
+       /* ... and that we didn't leave the update list for the tree */
+       if (ts->upd.node.leaf_p) {
                updt_locked = 1;
                HA_RWLOCK_WRLOCK(STK_TABLE_UPDT_LOCK, &t->updt_lock);
                if (HA_ATOMIC_LOAD(&ts->ref_cnt))
                        goto out_unlock;
        }
-       MT_LIST_DELETE(&ts->pend_updts);
        eb32_delete(&ts->exp);
        eb32_delete(&ts->upd);
        ebmb_delete(&ts->key);