mirrors/haproxy
1
0
Fork 0
mirror of https://git.haproxy.org/git/haproxy.git/ synced 2025-08-05 22:56:57 +02:00
Code Issues Projects Releases Wiki Activity

BUG/MEDIUM: stick-table: Decrement the ref count inside lock to kill a session

Browse Source 
When we try to kill a session, the shard must be locked before decrementing
the ref count on the session. Otherwise, the ref count can fall to 0 and a
purge task (stktable_trash_oldest or process_table_expire) may release the
session before we have the opportunity to acquire the lock on the shard to
effectively kill the session. This could lead to a double free.

Here is the scenario:

    Thread 1                                 Thread 2

  sktsess_kill(ts)
    if (ATOMIC_DEC(&ts->ref_cnt) != 0)
        return
                   /* here the ref count is 0 */

                                       stktable_trash_oldest()
                                          LOCK(&sh_lock)
                                          if (!ATOMIC_LOAD(&ts->ref_cnf))
                                              __stksess_free(ts)
                                          UNLOCK(&sh_lock)

                  /* here the session was released */
    LOCK(&sh_lock)
    __stksess_free(ts)  <--- double free
    UNLOCK(&sh_lock)

The bug was introduced in 2.9 by the commit 7968fe3889 ("MEDIUM:
stick-table: change the ref_cnt atomically"). The ref count must be
decremented inside the lock for stksess_kill() and sktsess_kill_if_expired()
function.

This patch should fix the issue #2611. It must be backported as far as 2.9. On
the 2.9, there is no sharding. All the table is locked. The patch will have to
be adapted.
This commit is contained in:
Christopher Faulet 2024-06-25 08:22:58 +02:00
parent bcf98c9b5f
commit 9357873641
2 changed files with 7 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
include/haproxy/stick_table.h
View File

@ -220,9 +220,6 @@ static inline void stksess_kill_if_expired(struct stktable *t, struct stksess *t
uint shard;
size_t len;
if (decrefcnt && HA_ATOMIC_SUB_FETCH(&ts->ref_cnt, 1) != 0)
return;
if (t->expire != TICK_ETERNITY && tick_is_expired(ts->expire, now_ms)) {
if (t->type == SMP_T_STR)
len = strlen((const char *)ts->key.key);
@ -235,9 +232,12 @@ static inline void stksess_kill_if_expired(struct stktable *t, struct stksess *t
ALREADY_CHECKED(shard);
HA_RWLOCK_WRLOCK(STK_TABLE_LOCK, &t->shards[shard].sh_lock);
__stksess_kill_if_expired(t, ts);
if (!decrefcnt || !HA_ATOMIC_SUB_FETCH(&ts->ref_cnt, 1))
__stksess_kill_if_expired(t, ts);
HA_RWLOCK_WRUNLOCK(STK_TABLE_LOCK, &t->shards[shard].sh_lock);
}
else if (decrefcnt)
HA_ATOMIC_SUB_FETCH(&ts->ref_cnt, 1);
}
/* sets the stick counter's entry pointer */

8
src/stick_table.c
View File

@ -170,10 +170,7 @@ int stksess_kill(struct stktable *t, struct stksess *ts, int decrefcnt)
{
uint shard;
size_t len;
int ret;
if (decrefcnt && HA_ATOMIC_SUB_FETCH(&ts->ref_cnt, 1) != 0)
return 0;
int ret = 0;
if (t->type == SMP_T_STR)
len = strlen((const char *)ts->key.key);
@ -186,7 +183,8 @@ int stksess_kill(struct stktable *t, struct stksess *ts, int decrefcnt)
ALREADY_CHECKED(shard);
HA_RWLOCK_WRLOCK(STK_TABLE_LOCK, &t->shards[shard].sh_lock);
ret = __stksess_kill(t, ts);
if (!decrefcnt || !HA_ATOMIC_SUB_FETCH(&ts->ref_cnt, 1))
ret = __stksess_kill(t, ts);
HA_RWLOCK_WRUNLOCK(STK_TABLE_LOCK, &t->shards[shard].sh_lock);
return ret;