BUG/MEDIUM: stktable: fix missing lock on some table converters

In 819fc6f563
("MEDIUM: threads/stick-tables: handle multithreads on stick tables"),
sample fetch and action functions were properly guarded with stksess
read/write locks for read and write operations respectively, but the
sample_conv_table functions leveraged by "table_*" converters were
overlooked.

This bug was not known to cause issues in existing deployments yet (at
least it was not reported), but due to its nature it can theorically lead
to inconsistent values being reported by "table_*" converters if the value
is being updated by another thread in parallel.

It should be backported to all stable versions.

[ada: for versions < 3.0, glitch_cnt and glitch_rate samples should be
 ignored as they first appeared in 3.0]

(cherry picked from commit 8919a80da9391e348aa325b44fdae40a35f48dcf)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 9f7e76bb0d738a9af1bac614e153995a9ba97d5e)
[cf: There is no bytes rate factor on 3.0]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

diff --git a/src/stick_table.c b/src/stick_table.c
index cf505bd..598a55d 100644 (file)
--- a/src/stick_table.c
+++ b/src/stick_table.c
@@ -1677,10 +1677,15 @@ static int sample_conv_table_bytes_in_rate(const struct arg *arg_p, struct sampl
                return 1;
 
        ptr = stktable_data_ptr(t, ts, STKTABLE_DT_BYTES_IN_RATE);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = read_freq_ctr_period(&stktable_data_cast(ptr, std_t_frqp),
                                                        t->data_arg[STKTABLE_DT_BYTES_IN_RATE].u);
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -1714,9 +1719,14 @@ static int sample_conv_table_conn_cnt(const struct arg *arg_p, struct sample *sm
                return 1;
 
        ptr = stktable_data_ptr(t, ts, STKTABLE_DT_CONN_CNT);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = stktable_data_cast(ptr, std_t_uint);
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -1750,9 +1760,14 @@ static int sample_conv_table_conn_cur(const struct arg *arg_p, struct sample *sm
                return 1;
 
        ptr = stktable_data_ptr(t, ts, STKTABLE_DT_CONN_CUR);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = stktable_data_cast(ptr, std_t_uint);
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -1786,10 +1801,15 @@ static int sample_conv_table_conn_rate(const struct arg *arg_p, struct sample *s
                return 1;
 
        ptr = stktable_data_ptr(t, ts, STKTABLE_DT_CONN_RATE);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = read_freq_ctr_period(&stktable_data_cast(ptr, std_t_frqp),
                                                        t->data_arg[STKTABLE_DT_CONN_RATE].u);
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -1899,10 +1919,15 @@ static int sample_conv_table_bytes_out_rate(const struct arg *arg_p, struct samp
                return 1;
 
        ptr = stktable_data_ptr(t, ts, STKTABLE_DT_BYTES_OUT_RATE);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = read_freq_ctr_period(&stktable_data_cast(ptr, std_t_frqp),
                                                        t->data_arg[STKTABLE_DT_BYTES_OUT_RATE].u);
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -1936,9 +1961,14 @@ static int sample_conv_table_glitch_cnt(const struct arg *arg_p, struct sample *
                return 1;
 
        ptr = stktable_data_ptr(t, ts, STKTABLE_DT_GLITCH_CNT);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = stktable_data_cast(ptr, std_t_uint);
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -1972,10 +2002,15 @@ static int sample_conv_table_glitch_rate(const struct arg *arg_p, struct sample
                return 1;
 
        ptr = stktable_data_ptr(t, ts, STKTABLE_DT_GLITCH_RATE);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = read_freq_ctr_period(&stktable_data_cast(ptr, std_t_frqp),
                                                        t->data_arg[STKTABLE_DT_GLITCH_RATE].u);
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -2012,9 +2047,14 @@ static int sample_conv_table_gpt(const struct arg *arg_p, struct sample *smp, vo
                return 1;
 
        ptr = stktable_data_ptr_idx(t, ts, STKTABLE_DT_GPT, idx);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = stktable_data_cast(ptr, std_t_uint);
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -2051,9 +2091,14 @@ static int sample_conv_table_gpt0(const struct arg *arg_p, struct sample *smp, v
        if (!ptr)
                ptr = stktable_data_ptr_idx(t, ts, STKTABLE_DT_GPT, 0);
 
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = stktable_data_cast(ptr, std_t_uint);
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -2090,9 +2135,14 @@ static int sample_conv_table_gpc(const struct arg *arg_p, struct sample *smp, vo
                return 1;
 
        ptr = stktable_data_ptr_idx(t, ts, STKTABLE_DT_GPC, idx);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = stktable_data_cast(ptr, std_t_uint);
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -2129,10 +2179,15 @@ static int sample_conv_table_gpc_rate(const struct arg *arg_p, struct sample *sm
                return 1;
 
        ptr = stktable_data_ptr_idx(t, ts, STKTABLE_DT_GPC_RATE, idx);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = read_freq_ctr_period(&stktable_data_cast(ptr, std_t_frqp),
                                                        t->data_arg[STKTABLE_DT_GPC_RATE].u);
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -2171,9 +2226,14 @@ static int sample_conv_table_gpc0(const struct arg *arg_p, struct sample *smp, v
                ptr = stktable_data_ptr_idx(t, ts, STKTABLE_DT_GPC, 0);
        }
 
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = stktable_data_cast(ptr, std_t_uint);
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -2207,15 +2267,25 @@ static int sample_conv_table_gpc0_rate(const struct arg *arg_p, struct sample *s
                return 1;
 
        ptr = stktable_data_ptr(t, ts, STKTABLE_DT_GPC0_RATE);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = read_freq_ctr_period(&stktable_data_cast(ptr, std_t_frqp),
                                                        t->data_arg[STKTABLE_DT_GPC0_RATE].u);
+
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
        else {
                /* fallback on the gpc array */
                ptr = stktable_data_ptr_idx(t, ts, STKTABLE_DT_GPC_RATE, 0);
-               if (ptr)
+               if (ptr) {
+                       HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                        smp->data.u.sint = read_freq_ctr_period(&stktable_data_cast(ptr, std_t_frqp),
                                                                t->data_arg[STKTABLE_DT_GPC_RATE].u);
+
+                       HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+               }
        }
 
        stktable_release(t, ts);
@@ -2256,9 +2326,14 @@ static int sample_conv_table_gpc1(const struct arg *arg_p, struct sample *smp, v
                ptr = stktable_data_ptr_idx(t, ts, STKTABLE_DT_GPC, 1);
        }
 
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = stktable_data_cast(ptr, std_t_uint);
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -2292,15 +2367,25 @@ static int sample_conv_table_gpc1_rate(const struct arg *arg_p, struct sample *s
                return 1;
 
        ptr = stktable_data_ptr(t, ts, STKTABLE_DT_GPC1_RATE);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = read_freq_ctr_period(&stktable_data_cast(ptr, std_t_frqp),
                                                        t->data_arg[STKTABLE_DT_GPC1_RATE].u);
+
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
        else {
                /* fallback on the gpc array */
                ptr = stktable_data_ptr_idx(t, ts, STKTABLE_DT_GPC_RATE, 1);
-               if (ptr)
+               if (ptr) {
+                       HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                        smp->data.u.sint = read_freq_ctr_period(&stktable_data_cast(ptr, std_t_frqp),
                                                                t->data_arg[STKTABLE_DT_GPC_RATE].u);
+
+                       HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+               }
        }
 
        stktable_release(t, ts);
@@ -2336,9 +2421,14 @@ static int sample_conv_table_http_err_cnt(const struct arg *arg_p, struct sample
                return 1;
 
        ptr = stktable_data_ptr(t, ts, STKTABLE_DT_HTTP_ERR_CNT);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = stktable_data_cast(ptr, std_t_uint);
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -2372,10 +2462,15 @@ static int sample_conv_table_http_err_rate(const struct arg *arg_p, struct sampl
                return 1;
 
        ptr = stktable_data_ptr(t, ts, STKTABLE_DT_HTTP_ERR_RATE);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = read_freq_ctr_period(&stktable_data_cast(ptr, std_t_frqp),
                                                        t->data_arg[STKTABLE_DT_HTTP_ERR_RATE].u);
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -2409,9 +2504,14 @@ static int sample_conv_table_http_fail_cnt(const struct arg *arg_p, struct sampl
                return 1;
 
        ptr = stktable_data_ptr(t, ts, STKTABLE_DT_HTTP_FAIL_CNT);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = stktable_data_cast(ptr, std_t_uint);
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -2445,10 +2545,15 @@ static int sample_conv_table_http_fail_rate(const struct arg *arg_p, struct samp
                return 1;
 
        ptr = stktable_data_ptr(t, ts, STKTABLE_DT_HTTP_FAIL_RATE);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = read_freq_ctr_period(&stktable_data_cast(ptr, std_t_frqp),
                                                        t->data_arg[STKTABLE_DT_HTTP_FAIL_RATE].u);
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -2482,9 +2587,14 @@ static int sample_conv_table_http_req_cnt(const struct arg *arg_p, struct sample
                return 1;
 
        ptr = stktable_data_ptr(t, ts, STKTABLE_DT_HTTP_REQ_CNT);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = stktable_data_cast(ptr, std_t_uint);
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -2518,10 +2628,15 @@ static int sample_conv_table_http_req_rate(const struct arg *arg_p, struct sampl
                return 1;
 
        ptr = stktable_data_ptr(t, ts, STKTABLE_DT_HTTP_REQ_RATE);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = read_freq_ctr_period(&stktable_data_cast(ptr, std_t_frqp),
                                                        t->data_arg[STKTABLE_DT_HTTP_REQ_RATE].u);
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -2555,9 +2670,14 @@ static int sample_conv_table_kbytes_in(const struct arg *arg_p, struct sample *s
                return 1;
 
        ptr = stktable_data_ptr(t, ts, STKTABLE_DT_BYTES_IN_CNT);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = stktable_data_cast(ptr, std_t_ull) >> 10;
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -2591,9 +2711,14 @@ static int sample_conv_table_kbytes_out(const struct arg *arg_p, struct sample *
                return 1;
 
        ptr = stktable_data_ptr(t, ts, STKTABLE_DT_BYTES_OUT_CNT);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = stktable_data_cast(ptr, std_t_ull) >> 10;
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -2627,9 +2752,14 @@ static int sample_conv_table_server_id(const struct arg *arg_p, struct sample *s
                return 1;
 
        ptr = stktable_data_ptr(t, ts, STKTABLE_DT_SERVER_ID);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = stktable_data_cast(ptr, std_t_sint);
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -2663,9 +2793,14 @@ static int sample_conv_table_sess_cnt(const struct arg *arg_p, struct sample *sm
                return 1;
 
        ptr = stktable_data_ptr(t, ts, STKTABLE_DT_SESS_CNT);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = stktable_data_cast(ptr, std_t_uint);
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -2699,10 +2834,15 @@ static int sample_conv_table_sess_rate(const struct arg *arg_p, struct sample *s
                return 1;
 
        ptr = stktable_data_ptr(t, ts, STKTABLE_DT_SESS_RATE);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = read_freq_ctr_period(&stktable_data_cast(ptr, std_t_frqp),
                                                        t->data_arg[STKTABLE_DT_SESS_RATE].u);
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }